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.

Sign up for GitHub

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

Jump to bottom

@azure/app-configuration: Fix docs for app config labelFilter #21039

Merged
merged 15 commits into from Mar 18, 2023
Merged

@azure/app-configuration: Fix docs for app config labelFilter #21039

merged 15 commits into from Mar 18, 2023

Conversation

rasmusbe
Copy link
Contributor

@rasmusbe rasmusbe commented Mar 25, 2022 •
edited

Packages impacted by this PR

@azure/app-configuration

Issues associated with this PR

Describe the problem that is addressed by this PR

When setting labelFilter in listConfigurationSettings to search for configs without any label it should be set to the null character \0 (it gets escaped to %00 by node-fetch). If it's set to %00 it will be escaped by node-fetch to %2500 and break the search.

What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen?

Are there test cases added in this PR? (If not, why?)

Only changes in docs

Provide a list of related PRs (if any)

Command used to generate this PR:**(Applicable only to SDK release request PRs)

Checklists

  • Added impacted package name to the issue description
  • Does this PR needs any fixes in the SDK Generator?** (If so, create an Issue in the Autorest/typescript repository and link it here)
  • Added a changelog (if necessary)

@ghost ghost added App Configuration Azure.ApplicationModel.Configuration customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Mar 25, 2022
@ghost
Copy link

ghost commented Mar 25, 2022

Thank you for your contribution rasmusbe! We will review the pull request and get back to you soon.

@rasmusbe rasmusbe changed the title Fix docs for app config labelFilter @azure/app-configuration: Fix docs for app config labelFilter Mar 25, 2022
@HarshaNalluru
Copy link
Member

Thanks for the PR @rasmusbe, I'll get back to you on Monday.
The pipelines are failing, will check that as well.

@rasmusbe
Copy link
Contributor Author

@HarshaNalluru any news on this?

@HarshaNalluru
Copy link
Member

HarshaNalluru commented May 11, 2022 •
edited

Thanks for waiting.

This fell off my radar as this wasn't tracked as part of the issues/milestones.

Adding a test as well to make sure this is working and to not ever break this feature for any reason such as changing the core stack to not depend on node-fetch.

@HarshaNalluru HarshaNalluru added this to the [2022] June milestone May 11, 2022
@HarshaNalluru HarshaNalluru self-assigned this May 11, 2022
HarshaNalluru
HarshaNalluru previously approved these changes May 12, 2022
View reviewed changes
@HarshaNalluru HarshaNalluru dismissed their stale review May 13, 2022 00:20

\0 works, but want to spend more time on this and get it digested with other relevant parties

@xirzec xirzec modified the milestones: [2022] June, [2022] July Jun 13, 2022
@xirzec xirzec modified the milestones: 2022-07, 2022-08 Jul 8, 2022
@xirzec xirzec modified the milestones: 2022-08, 2022-09 Aug 9, 2022
@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Oct 14, 2022
@ghost
Copy link

ghost commented Oct 14, 2022

Hi @rasmusbe. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@rasmusbe
Copy link
Contributor Author

@HarshaNalluru any news?

@ghost ghost removed the no-recent-activity There has been no recent activity on this issue. label Oct 14, 2022
@xirzec xirzec modified the milestones: 2022-09, 2022-12 Nov 1, 2022
@xirzec xirzec modified the milestones: 2022-12, 2023-02 Jan 11, 2023
@ghost ghost added the no-recent-activity There has been no recent activity on this issue. label Mar 17, 2023
@ghost
Copy link

ghost commented Mar 17, 2023

Hi @rasmusbe. Thank you for your interest in helping to improve the Azure SDK experience and for your contribution. We've noticed that there hasn't been recent engagement on this pull request. If this is still an active work stream, please let us know by pushing some changes or leaving a comment. Otherwise, we'll close this out in 7 days.

@rasmusbe
Copy link
Contributor Author

Well, It's still an issue but a bit shitty that it takes more than a year to fix the docs...

@ghost ghost removed the no-recent-activity There has been no recent activity on this issue. label Mar 17, 2023
HarshaNalluru
HarshaNalluru approved these changes Mar 17, 2023
View reviewed changes
Copy link
Member

@HarshaNalluru HarshaNalluru left a comment

Choose a reason for hiding this comment

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

Sorry for the delay @rasmusbe

@HarshaNalluru HarshaNalluru enabled auto-merge (squash) March 17, 2023 21:57
@HarshaNalluru HarshaNalluru merged commit e7b9dd1 into Azure:main Mar 18, 2023
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@HarshaNalluru HarshaNalluru HarshaNalluru approved these changes

Assignees

@HarshaNalluru HarshaNalluru

Labels
App Configuration Azure.ApplicationModel.Configuration customer-reported Issues that are reported by GitHub users external to the Azure organization.
Projects
None yet
Milestone
2023-02
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@rasmusbe @HarshaNalluru @xirzec