-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Introduce EditorConfig #242
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
Conversation
Is it needed to separate re-formatting files and introducing EditorConfig into two commits? |
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.
Mention EditorConfig in top-level documentation (i.e., README.md
).
Always minimize the necessary changes. |
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.
Rebase the latest master
branch.
Originally, the indentation of the scripts in directory 'scripts/' and the GitHub workflow was inconsistent due to human error, and some files contains lines with only spaces. This patch introduces EditorConfig, which is supported out-of-the-box by many editors, to help the auto-indent functionality work. It sets the indentation to the currently used indentation of each file respectively, and ask editors to use Unix-like newlines (line feed at the end of each line) and avoid trailing white-spaces. Change-Id: Idaa878cb779b637c1361baa637419645828530cb
Is it possible to enforce the style checks in GitHub Actions as well? |
There are some tools such as editorconfig-check, but I think the simplest way is to run a headless editor and diff the content before and after re-indenting, for example: cp $file $file.bak
nvim --headless +'norm vG=' +'x' $file
diff $file $file.bak However, the current scripts might not pass the test because it seems unable to handle "aligning" like VAR = $(command --flag arg |
another --flag arg)
^ These space characters let the commands aligned. |
It sounds great. Create an issue to enforce consistent style for the shell scripts after the merging of this pull request. |
Thank @lumynou5 for contributing! |
Also, EditorConfig is not language-aware. It doesn't know that Bash here document must not be indented with spaces (or not be indented at all). Therefore, rather than enforcing checks, I recommend to let it just help editors find correct indentation and avoid errors like before this PR, and human developers can align or do whatever making it more readable to humans. |
Originally, the indentation of the scripts was not unified: ones in 'scripts/' used 2 spaces, and ones in '.ci/' used 4. Even though the former used 2 spaces, the style was not implemented well as some indents in the files were 4 spaces.
This patch unifies the style of the scripts, and note it with an EditorConfig file. EditorConfig is used because it's supported out-of-the-box by many editors, helping the auto-indent functionality works.
Change-Id: I41be8779ef5b0e94192c144bb48605625727963f