-
Notifications
You must be signed in to change notification settings - Fork 698
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
Text file, newlines at end of files #9804
Conversation
@andreasabel created |
@ulysses4ever I see you are a fix-whitespace contributor. Do you think we can get |
bc51c4f
to
d5ed8f1
Compare
0a0b0fe
to
cc2372d
Compare
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.
You could also use https://github.com/andreasabel/fix-whitespace-action:
steps:
- uses: actions/checkout@v4
- uses: andreasabel/fix-whitespace-action@v1
This action downloads the pre-build binary of fix-whitespace
and runs it, and also caches it.
1c9385e
to
9435037
Compare
Thanks @andreasabel |
Label merge+no rebase is necessary when the pull request is from an organisation. |
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.
Instead of creating a separate workflow, can we add it to the quick-jobs workflow?
Can you check that fix-whitespace is run with a flag that will show the actual violations?
@ulysses4ever I checked and it shows violations. |
Very cool, thanks! |
@ulysses4ever, funny thing about moving it to quick jobs is that it now doesn't run nearly as quickly (the ghcup, hackage and alex stuff takes a lot longer to run)! |
In terms of developer feedback, putting it alongside (at the end of) quick jobs is a big degradation. |
Mm, good point. My intention was not having it run quickly, though, and rather I dislike proliferation of standalone workflows. But maybe it’d be good to get an actually quick feedback from this job… We need to fix quick-jobs to only download binaries and not build anything with ghc! |
@ulysses4ever I see there is something wrong with the cache in quick jobs;
|
Interesting... Maybe worth a new ticket with the continuous-integration label. |
- Fix whitespace in cabal-testsuite + other tests
Needed for POSIX compliance for text files
Co-authored-by: brandon s allbery kf8nh <allbery.b@gmail.com>
9435037
to
eb87ffd
Compare
How much squashing should I be doing with this given that the advice is:
|
Sorry @Mikolaj for the lack of squashing. I asked about squashing but forgot to come back to it. Noticed when I got the cabal.head email. |
Ouch. With no-rebase and no manual rebase nor squash just before merging, the 12 commits are strewn artistically across other PRs from the several last days. I guess we have to get used to git art. :D |
For the sake of bisecting, a linear history is really a plus. Can't we enforce it on this repo? (On the Agda repo, I disabled merge commits.) |
Some contributors — and most importantly, some employers — do not fancy At first we did some manual cherry-picking, but then we gave in and |
Can mergify do the rebase by copying the contributor branch to a new branch on this repo first? Can this be set up? This might be a technical solution. If a PR cannot be rebased then it should be rejected by default. Otherwise, we'll not get a linear history. Contributors with write access could be encouraged to push to a branch here rather than on their own repo. |
This could be difficult if the contributor is working on an employer's tree as part of their job. |
This requires a new issue for visibility and discussion. |
@mergify backport 3.12 |
✅ Backports have been created
|
* Add back whitespace workflow (cherry picked from commit 265f149) * Whitespace conventions in contributing (cherry picked from commit 7c9346b) * Fix whitespace in workflows (cherry picked from commit c31ca12) # Conflicts: # .github/workflows/validate.yml * Fix whitespace in docs (cherry picked from commit 2ea012e) * Fix whitespace in *.hs sources (cherry picked from commit e420bf2) * Add missing line at EOF for cabal-testsuite/**/*.hs - Fix whitespace in cabal-testsuite + other tests (cherry picked from commit e386a13) * Fix whitespace in fourmolu configuration (cherry picked from commit b790751) * Configure fix-whitespace to include *.project Needed for POSIX compliance for text files (cherry picked from commit ed4c36a) * Fix whitespace in *.project (cherry picked from commit 38eb1e8) * Ignore generated doc/buildinfo-fields-reference.rst (cherry picked from commit b414faf) * Reduce double space between prose words Co-authored-by: brandon s allbery kf8nh <allbery.b@gmail.com> (cherry picked from commit 834e1ed) * Use fix-whitespace-action (cherry picked from commit eb87ffd) * Fix merge conflict --------- Co-authored-by: Phil de Joux <philderbeast@gmail.com> Co-authored-by: Hécate <Kleidukos@users.noreply.github.com>
Apparently, we misunderstood each other. What I meant is that it should show not only the names of the files with whitespace issues but also the issues themselves. It could be done by the |
Fixes #9802 for
*.hs
and*.project
filesand adds a script taking a file glob pattern for doing this, adding newlines at the ends of files where needed. Adds back the whitespace github workflow and usesfix-whitespace
to fix non-conformance. Add a short note about this in the contributing readme.