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

[sort-imports] update agrifood-farming-rest with respect to sort-imports rule #19024

Closed
wants to merge 2 commits into from
Closed

[sort-imports] update agrifood-farming-rest with respect to sort-imports rule #19024

wants to merge 2 commits into from

Conversation

bzhang0
Copy link
Contributor

@bzhang0 bzhang0 commented Dec 8, 2021 •
edited
Loading

This PR is working in conjunction with other PRs to fix individual sections of the azure codebase with respect to the new sort-imports rule, as indicated in #9252

In this PR, I have fixed all files under sdk/agrifood/agrifood-farming-rest.

@bzhang0 bzhang0 requested a review from joheredi as a code owner December 8, 2021 00:43
@ghost ghost added AgriFood customer-reported Issues that are reported by GitHub users external to the Azure organization. labels Dec 8, 2021
@ghost
Copy link

ghost commented Dec 8, 2021

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

@ghost ghost added the Impact++ This pull request was submitted by a member of the Impact++ team. label Dec 8, 2021
@ghost
Copy link

ghost commented Dec 8, 2021

Thank you for your contribution, bzhang0!

@jeremymeng
Copy link
Member

@bzhang0 Sorry I probably didn't clarify. Some packages in our repo are auto-generated by codegen tool. This is one of them. Please ignore any packages that has -rest suffix in their names. I will also try to come up with a list of packages that are generated and share with you.

@bzhang0
Copy link
Contributor Author

bzhang0 commented Dec 8, 2021

@bzhang0 Sorry I probably didn't clarify. Some packages in our repo are auto-generated by codegen tool. This is one of them. Please ignore any packages that has -rest suffix in their names. I will also try to come up with a list of packages that are generated and share with you.

Oh, I see! I'll ignore them in that case. Still curious as to why I'm failing the tests, though.

@joheredi
Copy link
Member

joheredi commented Dec 8, 2021

@bzhang0 thanks for sending this PR out! How is this formatting being applied? Is it happening through the rushx format script? Or is it manual?

If this is coming from the rushx format script I think we should be good to take these changes in. The package.json defines the generate script as follows "generate:client": "autorest --typescript swagger/README.md && npm run format", which means that formatting will happen right after autorest generates the code

@bzhang0
Copy link
Contributor Author

bzhang0 commented Dec 8, 2021

Hey @joheredi! The first commit db53d96 was all manual (from running my own script), while the prettier commit 056e415 was simply running rushx format. Jeremy notified me that I shouldn't edit packages with the -rest suffix, so I believe this PR can just be ignored. Let me know if this is the case.

@ramya-rao-a
Copy link
Contributor

We can close this PR. We don't want to make any manual updates to packages that are purely autogenerated

@ramya-rao-a ramya-rao-a closed this Dec 8, 2021
@bzhang0 bzhang0 deleted the sort-imports--agrifood-farming-rest branch January 16, 2022 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joheredi joheredi Awaiting requested review from joheredi joheredi is a code owner

Assignees
No one assigned
Labels
AgriFood customer-reported Issues that are reported by GitHub users external to the Azure organization. Impact++ This pull request was submitted by a member of the Impact++ team.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bzhang0 @jeremymeng @joheredi @ramya-rao-a