Add .editorconfig to keep project files consistent #960
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
DEFRA/water-abstraction-team#115 We've been fortunate enough to grow as a team this year which means we've had several new developers join us! 🥳 Unfortunately, it has exposed our onboarding still leaves a lot to be desired. We find we keep having to highlight the same coding convention or stylistic issues in new members' initial PRs. This is irrespective of whether we have written up the convention or not. Wouldn't it be great if we used all this computing power we interact with daily to help them get it right the first time? That we help them fall into the [pit of success](https://www.hackterms.com/pit%20of%20success)? One idea is to add an [.editorconfig](https://editorconfig.org/) to the repo. Folks have the choice to add the extension (some IDEs will use it out of the box) but it at least defines some basics for any code files folks wish to add. This change adds an initial version generated using the [VSCode extension](https://marketplace.visualstudio.com/items?itemName=EditorConfig.EditorConfig) based on current editor settings.
Cruikshanks
added
the
housekeeping
Though it has the most stars maintenance for [EditorConfig-Action](https://github.com/marketplace/actions/editorconfig-action) seems to have stopped. People have been pushing fixes but no one is merging them in. When we tried this action we got ``` ERROR: failed to solve: process "/bin/sh -c npm install --no-save . && \tln -s $(npm bin)/eclint /usr/local/bin && \tnpm audit && \techo \"eclint version: $(eclint --version)\"" did not complete successfully: exit code: 1 Warning: Docker build failed with exit code 1, back off 7.523 seconds before retry. ``` So, we're trying the next most popular result from the marketplace!
Nothing got flagged by the checker. So, following docs even though VSCode is screaming that `run:` shouldn't be there!
Now the checker seems to be picking up the issues
By going through the files VSCode displays the line endings for the current file in the status bar. Switching that from `CRLF` to `LF` cause git to see a change though there is nothing discernable from the editor itself!
It seems 2, 4, 6 etc indents are not picked up but 1, 3, 5 etc are.
This one doesn't require both a `use`` and a `run` step.
It might require 2 steps, but at least it is dependent on a maintained editorconfig linter. The rest of the options in the marketplace seemed to be dependent on ECLint which is now archived.
|
This looks like a scary amount of changes. But when you go to look at the files, close the
Adding this and the CI action highlighted that the migration files had inconsistent line endings. I've corrected them in this change to get CI to pass and to start with no errors. But that means Git thinks I've changed thousands of lines of code! |
Cruikshanks
requested review from
Demwunz,
robertparkinson,
Jozzey,
jonathangoulding,
Beckyrose200 and
rvsiyad
Cruikshanks
changed the title
There was a problem hiding this comment.
Looks like it works for me in Webstorm.
I think we merge this to main then check with eslint branch to make sure they are consistent and not fighting each other. ( I will hold off the eslint branch merge until this is in)
We can investigate the use of https://www.npmjs.com/package/eslint-plugin-editorconfig if necessary.
|
Agreed @jonathangoulding . I'm not over the moon about the CI choice/option. There seems little support for using a stock GitHub action when it comes to linting based on your .editorconfig. But that might be because everyone is using their stock linter + plugin/extension like you suggest. So, defo once this and the ESlint branch are merged we should look into whether eslint-plugin-editorconfig is a better solution. |
Jozzey
approved these changes
May 1, 2024
Beckyrose200
approved these changes
May 1, 2024
Cruikshanks
added a commit
to DEFRA/water-abstraction-service
that referenced
this pull request
Nov 21, 2024
https://eaflood.atlassian.net/browse/WATER-4729 When working on updating the environment to Node v22 we encountered an error with the `entrypoint.sh`. Docker compose couldn't find the file when rebuilding the new image. Huh? We know! Countless numbers of us have built this environment successfully, but the error was there plain and clear. Perhaps its a change in docker compose? Anyway, miraculously one of the team tracked it down to the file having a mix of `LF` and `CRLF` line endings. Once they updated the line endings in the file to always be `LF`, docker compose succeeded in building the image. So, this change corrects all files with mixed line endings. We then add a [.editorconfig](https://editorconfig.org/) file to the repo. We did this to [water-abstraction-system](DEFRA/water-abstraction-system#960) as part of helping to ensure consistency in our code and the same applies here.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
DEFRA/water-abstraction-team#115
We've been fortunate enough to grow as a team this year which means we've had several new developers join us! 🥳
Unfortunately, it has exposed our onboarding still leaves a lot to be desired. We find we keep having to highlight the same coding convention or stylistic issues in new members' initial PRs. This is irrespective of whether we have written up the convention or not.
Wouldn't it be great if we used all this computing power we interact with daily to help them get it right the first time? That we help them fall into the pit of success?
One idea is to add an .editorconfig to the repo. Depending on folks IDE choice it will either be recognised out-of-the-box or they'll need an extension.
Unlike other tools having the file doesn't do anything directly to your project. Instead, it tells your chosen editor how to behave when working with the files. For example, when you indent something whether to use tabs or spaces and how many. If your editor can remove whitespace on save, the
.editorconfigwill tell it whether to do that or not.Even for folks using an incompatible editor or who choose not to install the extension, we at least have something that defines what we expect and can be used in our CI pipeline.
This change adds an initial version generated using the VSCode extension based on current editor settings.
It also updates our GitHub CI workflow to check that pushed changes match our conventions.
This change highlighted that the line endings in our migrations were inconsistent. So, the fix for those is included.