Autoformat more directories #10491
Autoformat more directories #10491
Conversation
|
bold! People seem to want to scrap auto-formatting as it creates a noticeable tension with new/rare contributors. One thing to consider is the release cycle. We're about to release cabal-install-3.14.1.0, and there will be many backports, which reformatting may complicate. A better point for it may be a little while after a release. |
|
That's likely to feel like 3.10 after the initial fourmolu pass, though. |
FWIW I was very surprised to hear this -- I find an autoformatter that clearly fails in CI so much easier to work with as an infrequent contributor than a (usually vague) style guide that's variably enforced by individual reviewers... |
|
I'd really like to see this go in. I don't want have to bother formatting code by hand. |
|
@9999years, I can help with the CPP drudgery if that would speed things up. |
Since we turned 180 on scrapping the autoformatter (again!) and seem to be sticking with it, I'd be fine with this patch (once it's turned out of the draft state), in principle. Is it intentional that it doesn't touch the CI? CI currently does its own thing with the action, and has its own list of directories, sadly. I'd be happier if CI exercised the make-targets. But I also foresee some contributors screaming at CI if it starts checking tests, for instance... |
|
@ulysses4ever Definitely not intentional, good catch! |
9999years
force-pushed
the
format-more
branch
3 times, most recently
from
5f18808 to
f7a7a15
Compare
9999years
force-pushed
the
format-more
branch
from
f7a7a15 to
0ecda25
Compare
.github/workflows/format.yml
Outdated
| Cabal-testsuite/src/**/*.hs | ||
| Cabal-testsuite/main/**/*.hs | ||
| Cabal-testsuite/static/**/*.hs |
There was a problem hiding this comment.
Case seems off here (and maybe somewhere else)? I mean Cabal-testsuite not cabal-testsuite, and the latter is the correct one.
It's also strange that CI missed this issue.
|
@alt-romes @mpickering Do you have any PRs which would conflict with this? Happy to split this PR up. |
|
From today's meeting: We might want to pay attention to: #9743 |
|
To expand on @9999years's comment above, we're particularly worried about #9743 But as discussed at the meeting, the PR branch could be reformatted accordingly, and then it should hopefully not require any other effort. |
|
If we're worried about conflicts, we could merge #10683 early that deals with the parsing errors fourmolu encounters, then merge other major pull requests that we don't want to reformat and then merge this pull request. |
|
I don't think that a autoformatter should be used at all, so splitting up the PR or anything else won't help so much. I thought the consensus was to remove the formatter and it seems that a single PR has changed the course again? There will always be long-lived branches for large open-source projects which become hard to rebase, it's not a temporary problem. |
|
@mpickering The current plan is to autoformat the LTS branches to match this PR, once this PR is merged, so that backports will remain easy. Does that help ameliorate your concerns? |
|
@9999years No, that does not address the concern at all. The concern is that there are long-lived contributor branches for complicated features which may exists for years outside the tree before being merged. |
|
@mpickering you can reformat the long-lived branch any time, if the "base" branch you want to merge into was reformatted over time. And the result would be the diff we are interested in, nothing about formatting. I still don't see any problem. What am I missing? |
sickos yes