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.

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...) #1825

Closed
kirthik opened this issue Feb 14, 2017 · 7 comments
Assignees

Comments

@kirthik
Copy link
Contributor

kirthik commented Feb 14, 2017

We need a linter rule for catching this one.

Moving from Azure/azure-rest-api-specs#380

cc @lvh

@veronicagg
Copy link
Contributor

is AutoRest doing this already? do we need to implement this in CI?

@dsgouda
Copy link
Contributor

dsgouda commented Apr 21, 2017

Wonder if this should be linterized or should AutoRest automatically format the file before processing. @olydis thoughts?

@fearthecowboy
Copy link
Member

UTF-8 is all we produce.

We enforce LF line endings for certain extensions already, the rest are assumed to be CRLF. If there is one producing something not consistent with expectations, that can be updated.

G

@lvh
Copy link

lvh commented Apr 23, 2017

@fearthecowboy The original linked issue (#380) shows a counterexample: re UTF-8 and LF/CRLF endings.

@veronicagg My suggestion for CI is mostly because the stuff you need to do to detect the problem is basically most of the work you need to do to get it in CI anyway, so I'd suggest doing it in CI to make sure that there are no regression instead of fixing it once :) The linked issue give some examples of commands you may want to run in CI; specifically, I'd suggest eclint and iconv.

I don't know what editor is popular among people who work on this but VS Code has a plugin for Editorconfig. https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig

@veronicagg
Copy link
Contributor

@vishrutshah is this something you think could be added to CI?

@veronicagg veronicagg removed the linter label Jun 8, 2017
@veronicagg veronicagg removed this from the Linter rules wave 5 milestone Jun 8, 2017
@vishrutshah
Copy link
Contributor

Based on the tool mentioned in the linked PR, Check is doable. I'd not prefer to correct those changes as part of the CI though.

@kirthik This should be moved back to the azure-rest-api-spec repo instead of autorest. Let me know if you think otherwise then i can reopen the previous issue and close this one.

@fearthecowboy
Copy link
Member

reactivated in original repo

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants