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

Test CI formatting check #73

Closed
wants to merge 12 commits into from
Closed

Test CI formatting check #73

wants to merge 12 commits into from

Conversation

emdash-ie
Copy link
Contributor

@emdash-ie emdash-ie commented Sep 30, 2024

This PR makes a number of weird formatting changes, to test whether the CI is checking formatting.

Indented this line with a tab, the rest of this file is indented with
spaces.
@emdash-ie emdash-ie changed the title Test long line Test CI formatting check Sep 30, 2024
@emdash-ie
Copy link
Contributor Author

As shown by the build failures on the three commits, our CI is not checking the formatting of our files with our prettier config. Whatever checking it is doing appears insufficient, as well – I think we would want our formatting check to at least complain about mixed tabs-and-spaces and negative indentation of continued lines.

@emdash-ie
Copy link
Contributor Author

Added two more commits, showing the CI check is not complaining about odd spacing in the middle of the line (including a tab!), nor about trailing space at the ends of lines.

@emdash-ie
Copy link
Contributor Author

emdash-ie commented Sep 30, 2024

As a test, I’ve cherry-picked the commits from #65 that originally configured eslint to run prettier. The build failure shows that this successfully runs prettier with our config.

I copied these [from
dotcom-rendering](https://github.com/guardian/dotcom-rendering/blob/f9f1c28066c048a1b4c281514d69382f220d9a19/package.json#L19-L20)
– I figure it’s probably good to be consistent with how they do things.

In particular, I’m adding these because I can’t get nx to run prettier
with the same config as when I run it directly (I don’t even know if
it’s definitely running prettier!): running `npm run format:check` with
these changes warns that 102 files need formatting, whereas running `npm
run nx format:check` only says two files need formatting
(package-lock.json and package.json).

Cherry picked from commit 4dfe61b.
To ensure the files are formatted with prettier, this commit runs the
format:check script in the github actions build.

Cherry picked from commit 1e57fe7.
`npm ci` installs the dependencies, e.g. prettier, so the format check
fails because prettier can’t be found!

Cherry picked from commit 9ae4fd1.
@emdash-ie
Copy link
Contributor Author

As a second test, I reverted the eslint config and restored the check that runs prettier directly – that also fails the build, correctly detecting the formatting errors.

@emdash-ie
Copy link
Contributor Author

Closing this PR as it was just for testing

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

Successfully merging this pull request may close these issues.

2 participants