-
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
CI should verify that files are appropriately formatted (UTF-8, newlines, whitespace...) #380
Comments
This also appears to be an issue in other PRs, e,g #379. It seems like giving people a tool to do this automagically would save a ton of developer and reviewer time :) |
Moved to - Azure/autorest#1825 |
Reopening -- this is a CI issue, not autorest |
@bsiegel not sure if we have this already, but seems like a good investment if we don't. |
I would suggest use a gitattribute files to enforce all JSON to be LF.
|
We can go further, it's pretty easy to add the editorconfig file and then add a pre-commit hook that runs the files through eclint first. The only downside is that we'd need to either commit the tool to the repo or people would have to run 'npm install' locally to pull it down. |
I don’t know why this was closed: the associated PR does not appear to resolve the problem that existing JSON files are in disarray, and CI does not prevent regressions. |
HI @NullMDR could you pls have a look? before adding check, pls see how to fix existing errors. thanks. |
Hi @NullMDR, is this fixed? |
We already have prettier that could do format, but for the newline and utf-8 I need to confirm if it works with current prettier. |
@PhoenixHe-msft any update on this thread? |
Closing as this issue is too old. If this is still an issue, we should open a new issue on the tooling. |
In #372, I originally suggested that maybe CI should do:
However, this is not the only inconsistency I tripped over. For example, at time of writing,
arm-devtestlabs/2016-05-15/swagger/DTL.json
contained CRLF line endings (other files use LF).One way to do both enforce, document and automate all of this is an
.editorconfig
file. This lets you specify:Many editors support this either natively or with a plugin. This includes the Github built in editor, Emacs, vim, Visual Studio, and Visual Studio Code.
Here's an example config:
This would cover all the inconsistencies I have found in the specs so far. Using eclint, you can automatically both check and even fix all current issues; so it's easy to move to this and it's easy to fix existing issues.
@John-Hart, @lmazuel: If I write this PR, is there any chance it gets merged?
The text was updated successfully, but these errors were encountered: