-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[core-client] Prepare release for v1.3.3 out of a hotfix branch #18856
[core-client] Prepare release for v1.3.3 out of a hotfix branch #18856
Conversation
* Check is value is undefined in appendQueryParams * Minor change * Response to PR comments * Update sdk/core/core-client/src/urlHelpers.ts Co-authored-by: Jeff Fisher <xirzec@xirzec.com> * Minor refactor Co-authored-by: Jeff Fisher <xirzec@xirzec.com>
Post release automated changes for azure-core-client
We have the word "Experimental" in the titles in the readme files for our core packages. My guess is that we started with this when we were working on core v2, forgot to remove it when core v2 went GA and all our newer core packages from them did a copy/paste :) @xirzec, @joheredi Am I missing something?
* Update Changelog to include query param check * Update sdk/core/core-client/CHANGELOG.md Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com> Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com>
// QUIRK: If we get an array of values, include multiple key/value pairs | ||
for (const subValue of value) { | ||
searchPieces.push(`${name}=${subValue}`); | ||
} | ||
} else { | ||
searchPieces.push(`${name}=${value}`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, is this designed to handle undefined? Wouldn’t it add undefined
as the string value here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, it would add undefined
as the literal value
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is that intentional? if so, disregard.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some context: #18621 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sarangan12 perhaps consider creating a test case for this behavior to document it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wait, shouldn’t we skip it? would it work too or would it fail? meaning, to remove things like C=undefined
. The link in context indicates that C=undefined
works, but it does not indicate that removing that query property would fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a little torn, since silently removing the query params seems like a slightly larger jump than filling in a bogus value. @sarangan12 did your investigation yield if the presence of this parameter mattered at all?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@xirzec Yes. Removing the value will cause the error if the value is not an optional value.
API changes have been detected in API changes - export declare function authorizeRequestOnClaimChallenge(onChallengeOptions: AuthorizeRequestOnChallengeOptions): Promise<boolean>; |
API changes have been detected in |
API changes have been detected in API changes - toJson(options?: {
- preserveCase?: boolean;
- }): RawHttpHeaders;
+ toJson(): RawHttpHeaders;
- toJson(options?: {
- preserveCase?: boolean;
- }): RawHttpHeaders;
+ toJson(): RawHttpHeaders; |
API changes have been detected in API changes - logger?: AzureLogger;
- logger?: AzureLogger;
- logger?: AzureLogger;
- toJSON(options?: {
- preserveCase?: boolean;
- }): RawHttpHeaders;
+ toJSON(): RawHttpHeaders; |
* Generate absolute path for symlink to reinstall native dependency
…) (#18925) * Check is value is undefined in appendQueryParams (#18621) * Check is value is undefined in appendQueryParams * Minor change * Response to PR comments * Update sdk/core/core-client/src/urlHelpers.ts Co-authored-by: Jeff Fisher <xirzec@xirzec.com> * Minor refactor Co-authored-by: Jeff Fisher <xirzec@xirzec.com> * Post release automated changes for core releases (#18358) Post release automated changes for azure-core-client * Remove the word experimental in readmes (#18773) We have the word "Experimental" in the titles in the readme files for our core packages. My guess is that we started with this when we were working on core v2, forgot to remove it when core v2 went GA and all our newer core packages from them did a copy/paste :) @xirzec, @joheredi Am I missing something? * Update Changelog to include query param check (#18851) * Update Changelog to include query param check * Update sdk/core/core-client/CHANGELOG.md Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com> Co-authored-by: Deyaaeldeen Almahallawi <dealmaha@microsoft.com> * fix reinstall native dependency (#18582) * Generate absolute path for symlink to reinstall native dependency Co-authored-by: Sarangan Rajamanickam <sarajama@microsoft.com> Co-authored-by: Jeff Fisher <xirzec@xirzec.com> Co-authored-by: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com> Co-authored-by: praveenkuttappan <55455725+praveenkuttappan@users.noreply.github.com> Co-authored-by: Sarangan Rajamanickam <sarajama@microsoft.com> Co-authored-by: Jeff Fisher <xirzec@xirzec.com> Co-authored-by: Azure SDK Bot <53356347+azure-sdk@users.noreply.github.com> Co-authored-by: Ramya Rao <ramya.rao.a@outlook.com> Co-authored-by: praveenkuttappan <55455725+praveenkuttappan@users.noreply.github.com>
Azure Event Grid 2021-12-01: Add missing isDataAction property from operation resource to address s360 LPI (Azure#18856) * Fix missing property * fix issecret Co-authored-by: Ashraf Hamad <ahamad@ntdev.microsoft.com>
This PR merges a bugfix to a hotfix branch where v1.3.3 will be released from. This is needed so a recently added new feature does not get released in December.