Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure az_json_writer_append_json_text appends a comma between elements of an array. #1600

Merged
merged 3 commits into from
Feb 12, 2021

Conversation

ahsonkhan
Copy link
Contributor

Fixes #1574

@ahsonkhan
Copy link
Contributor Author

cc @kevinior

@ahsonkhan ahsonkhan requested a review from joshfree as a code owner February 11, 2021 21:52
@@ -1,7 +1,10 @@
# Release History

## 1.1.0-beta.2 (Unreleased)
## 1.1.0-beta.4 (Unreleased)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was updated to be one higher than the version in the iot_pnp branch which is where earlier betas were shipped out of.
https://github.com/Azure/azure-sdk-for-c/blob/93fb3362a37753c67aca4b52b852441e097f4dbf/CHANGELOG.md#110-beta4-unreleased

FYI @danewalton, when you do your next sync.

@ahsonkhan
Copy link
Contributor Author

@sima-zhu can you please help me understand where this broken link issue is coming from as part of the Link verification check and why it is getting flagged in this PR when it isn't modifying that markdown at all (and not in previous CI runs)?
https://dev.azure.com/azure-sdk/public/_build/results?buildId=732609&view=logs&j=a129effc-2dd1-54d1-fb5a-ad7bdc0e851d&t=ccad100f-2904-5913-2083-c525ad929a03&l=40

Summary of broken links:
'file:///D:/a/_work/1/s/sdk/samples/iot/docs/how_to_iot_hub_realtek_amebaD.md' has 1 broken link(s):
  ***/blob/master/sdk/samples/iot/aziot_realtek_amebaD
##[error]Found 222 links with 1 page(s) broken.
##[error]PowerShell exited with code '1'.

Copy link
Member

@vhvb1989 vhvb1989 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this mean we would want to release next March to publish this bug fix?

@ahsonkhan
Copy link
Contributor Author

Does this mean we would want to release next March to publish this bug fix?

I think so. But I will leave it up to Rick to decide, especially if we take it as a hot-fix for 1.0.1.

@RickWinter
Copy link
Member

@sima-zhu can you please help me understand where this broken link issue is coming from as part of the Link verification check and why it is getting flagged in this PR when it isn't modifying that markdown at all (and not in previous CI runs)?
https://dev.azure.com/azure-sdk/public/_build/results?buildId=732609&view=logs&j=a129effc-2dd1-54d1-fb5a-ad7bdc0e851d&t=ccad100f-2904-5913-2083-c525ad929a03&l=40

Summary of broken links:
'file:///D:/a/_work/1/s/sdk/samples/iot/docs/how_to_iot_hub_realtek_amebaD.md' has 1 broken link(s):
  ***/blob/master/sdk/samples/iot/aziot_realtek_amebaD
##[error]Found 222 links with 1 page(s) broken.
##[error]PowerShell exited with code '1'.

cc: @ewertons the link failure you had is showing up in master. What was the fix you applied in your PR?
#1568

@ahsonkhan
Copy link
Contributor Author

@sima-zhu, @weshaggard - why is the generate release artifact passing here. Shouldn't the link verification step catch the same issue. I don't understand the fickleness
#1602

https://dev.azure.com/azure-sdk/public/_build/results?buildId=733883&view=logs&j=a129effc-2dd1-54d1-fb5a-ad7bdc0e851d&t=ccad100f-2904-5913-2083-c525ad929a03
image

@ahsonkhan
Copy link
Contributor Author

OK, I think it was because the parent of my branch was several months old. Hopefully the merge from master will fix the issue:

commit 09617f2e7311f61ff32293a5e55aa55129361248
Author: Ahson Khan <ahson_ahmedk@yahoo.com>
Date:   Fri Oct 9 14:26:05 2020 -0700

    Add helper comment in clang format validation step to let contributor know                                                                                                                                                                                                 how to fix it (#1361)

@ahsonkhan
Copy link
Contributor Author

Alright, syncing with master resolved the issue. My branch was just out-of-sync, and there is no link verification issue.

@ahsonkhan ahsonkhan merged commit df15084 into Azure:master Feb 12, 2021
@ahsonkhan ahsonkhan deleted the FixAppendText branch February 12, 2021 21:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

az_json_writer_append_json_text doesn't add comma when needed
3 participants