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

remove en-us from links #12697

Closed
wants to merge 12 commits into from
Closed

remove en-us from links #12697

wants to merge 12 commits into from

Conversation

KarishmaGhiya
Copy link
Member

  • update swagger that generates the links
  • do not update yaml files

@KarishmaGhiya KarishmaGhiya mentioned this pull request Jul 23, 2020
Merged
@KarishmaGhiya
Copy link
Member Author

Moving comments from a different PR -
sdk/compute/azure-mgmt-imagebuilder/azure/mgmt/imagebuilder/models/_models.py - L542
Changing generated code has no value, since the next generation will put it back again.
I would suggest to make a list of mgmt packages that have those kind of issues, and open Swagger bugs instead.

For instance, the root cause of this one is here:
https://github.com/Azure/azure-rest-api-specs/blob/900ef530bda427a6ba9f61ac6f64952ea7650a37/specification/imagebuilder/resource-manager/Microsoft.VirtualMachineImages/stable/2020-02-14/imagebuilder.json#L579

@KarishmaGhiya KarishmaGhiya self-assigned this Jul 23, 2020
@KarishmaGhiya KarishmaGhiya added the EngSys This issue is impacting the engineering system. label Jul 23, 2020
iscai-msft
iscai-msft reviewed Jul 24, 2020
View reviewed changes
sdk/batch/azure-batch/azure/batch/models/_models.py Outdated
@@ -63,7 +63,7 @@ class AccountListSupportedImagesOptions(Model):

:param filter: An OData $filter clause. For more information on
constructing this filter, see
https://docs.microsoft.com/rest/api/batchservice/odata-filters-in-batch#list-support-images.
https://docs.microsoft.com/en-us/rest/api/batchservice/odata-filters-in-batch#list-support-images.
Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, just confused why you're adding back 'en-us' in this commit, lmk if I'm missing ontext

Copy link
Member Author

Choose a reason for hiding this comment

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

#12697 (comment) This code is autogenerated so it won't make a difference if i change here, it will be overridden

lmazuel
lmazuel requested changes Jul 24, 2020
View reviewed changes
Copy link
Member

@lmazuel lmazuel left a comment

Choose a reason for hiding this comment

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

You are removing 189.842 lines! :o
Looking closely, in some cases you actually remove recordings in this PR. There should be no YAML files touched at all in this PR. There is way in git to play with sub-folder checkout to make sure of that.

weshaggard
weshaggard reviewed Jul 24, 2020
View reviewed changes
eng/common/TestResources/New-TestResources.ps1
@@ -425,7 +425,7 @@ test resources (e.g. Role Assignments on resources). It is passed as to the ARM
template as 'testApplicationOid'

For more information on the relationship between AAD Applications and Service
Principals see: https://docs.microsoft.com/en-us/azure/active-directory/develop/app-objects-and-service-principals
Principals see: https://docs.microsoft.com/azure/active-directory/develop/app-objects-and-service-principals
Copy link
Member

Choose a reason for hiding this comment

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

This is going to need to be changed in the azure-sdk-tools repo.

@weshaggard
Copy link
Member

@KarishmaGhiya lets limit the changes here to rst and md files. The yaml files are test recordings and we cannot change them without breaking the tests. The py files are mostly generated so changing them will do no good without updating the source where they come from. After chatting with @lmazuel he suggested we instead try to update these links in the azure-rest-api-specs repo which contains the swagger files which is where these come from.

As part of doing that lets please reset the branch to one commit to avoid a build storm, ping me if you need help doing that.

@KarishmaGhiya
Copy link
Member Author

Closed this in favour of PR #13102

@KarishmaGhiya KarishmaGhiya deleted the remove-locale branch August 13, 2020 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@weshaggard weshaggard weshaggard left review comments

@iscai-msft iscai-msft iscai-msft left review comments

@lmazuel lmazuel lmazuel requested changes

@aim-for-better aim-for-better Awaiting requested review from aim-for-better

@amishra-dev amishra-dev Awaiting requested review from amishra-dev

@annatisch annatisch Awaiting requested review from annatisch

@bgklein bgklein Awaiting requested review from bgklein

@chlowell chlowell Awaiting requested review from chlowell

@danieljurek danieljurek Awaiting requested review from danieljurek

@idear1203 idear1203 Awaiting requested review from idear1203

@johanste johanste Awaiting requested review from johanste

@KieranBrantnerMagee KieranBrantnerMagee Awaiting requested review from KieranBrantnerMagee

@kristapratico kristapratico Awaiting requested review from kristapratico

@mayurid mayurid Awaiting requested review from mayurid

@mitchdenny mitchdenny Awaiting requested review from mitchdenny

@rakshith91 rakshith91 Awaiting requested review from rakshith91

@scbedd scbedd Awaiting requested review from scbedd

@schaabs schaabs Awaiting requested review from schaabs

@southpolesteve southpolesteve Awaiting requested review from southpolesteve

@xiafu-msft xiafu-msft Awaiting requested review from xiafu-msft

@xiangyan99 xiangyan99 Awaiting requested review from xiangyan99

@xingwu1 xingwu1 Awaiting requested review from xingwu1

@YijunXieMS YijunXieMS Awaiting requested review from YijunXieMS

@yunhaoling yunhaoling Awaiting requested review from yunhaoling

@zezha-msft zezha-msft Awaiting requested review from zezha-msft

@zfoster zfoster Awaiting requested review from zfoster

@zikalino zikalino Awaiting requested review from zikalino

Assignees

@KarishmaGhiya KarishmaGhiya

Labels
EngSys This issue is impacting the engineering system.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@KarishmaGhiya @weshaggard @lmazuel @iscai-msft