-
Notifications
You must be signed in to change notification settings - Fork 5.1k
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
Replace invalid-UTF-8 smart quotes #372
Conversation
Hi @lvh, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution! TTYL, AZPRBOT; |
The build is failing on generating the compute client, which doesn't make a lot of sense to me. I could remove those commits, but it appears the build only looks at files in the PR, so that might simply silence the failure... Could someone else take a look to confirm? It looks like it might just be downloading the wrong Python package for codegen: https://travis-ci.org/Azure/azure-rest-api-specs/jobs/140436786#L250 |
Hi @lvh The build breaks because of this bug: The current compute file is invalid, but no-one has produced a PR with the fix right now, so any PR with a compute modification will failed until someone fixes this bug. If you have the opportunity to fix #352, it will be awesome :). You can push it here at the same time. Thank you, |
Thanks for the heads up @lmazuel. I thought the clients were generated from this Swagger. Is that not true, or has this caused a halt in the development of e.g. the Python Azure library? |
@lvh We added a few new verifications of the Swagger validity and found a problem in the compute file. Since this new version of Autorest, the Python extension is no more able to generate the compute client. |
I'm working on getting the CLA signed; I have re-applied the fixes to compute on top of master. |
Hi @lvh, I'm your friendly neighborhood Azure Pull Request Bot (You can call me AZPRBOT). Thanks for your contribution! TTYL, AZPRBOT; |
(Oops; misclicked -- I did not intend to do that.) |
@@ -116,7 +116,7 @@ | |||
"in": "query", | |||
"required": false, | |||
"type": "string", | |||
"description": "The filter to apply on the operation. It ONLY supports the �eq� and �and� logical operators at this time. All the 4 query parameters 'OfferDurableId', 'Currency', 'Locale', 'Region' are required to be a part of the $filter." |
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 have no idea why the diff generator thinks eq
changed but and
didn't :)
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.
@lvh It's showing both 'eq' and 'and' changed, is that not what you expected?
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, I changed both of the quotes. But the diff generator thinks the word itself changed, which it didn't. I checked in the commit diff; it's just a GitHub display issue.
@lvh, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR. |
Looks Good |
No modification for NodeJS |
No modification for Python |
No modification for NodeJS |
These bytes (0x92, 0x93), which appear to be intended as smart quotes, are invalid UTF-8, preventing it from being parsed by many tools. I have replaced them with single quotes (Unicode APOSTROPHE) to match the surrounding style. This is the only file with this defect.
Also cleaned up some end-of line whitespace.
I have found that most files do not end in newlines. Is that intentional?
Should you be interested in automated tooling that can help find this, here's the (fish, but will work in zsh and possibly recent bash) command I used to check that there was only one file:
I don't have a CLA; could you please send me in the right direction for submitting one?