Skip to content

Add a note for contributors installing fourmolu #10473

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Oct 27, 2024
Merged

Add a note for contributors installing fourmolu #10473

merged 1 commit into from
Oct 27, 2024

Conversation

philderbeast
Copy link
Collaborator

Fixes #10472.

Template B: This PR does not modify behaviour or interface

E.g. the PR only touches documentation or tests, does refactorings, etc.

Include the following checklist in your PR:

  • Patches conform to the coding conventions.
  • Is this a PR that fixes CI? If so, it will need to be backported to older cabal release branches (ask maintainers for directions).

@philderbeast philderbeast force-pushed the doc/install-fourmolu-10472 branch 2 times, most recently from 9e9abe5 to e2f28ea Compare October 23, 2024 15:29
geekosaur
geekosaur requested changes Oct 23, 2024
View reviewed changes
Copy link
Collaborator

@geekosaur geekosaur left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trying to be a bit more formal, and include a way to install with a specific ghc. (Some people involved with cabal don't trust --with-compiler, but I use it somewhat regularly.)

@philderbeast philderbeast force-pushed the doc/install-fourmolu-10472 branch 3 times, most recently from 85ba5b8 to 2493075 Compare October 23, 2024 20:41
@philderbeast
Copy link
Collaborator Author

@geekosaur I've squashed your suggestions and moved the --with-compiler option to a tip.

ulysses4ever
ulysses4ever reviewed Oct 23, 2024
View reviewed changes
jasagredo
jasagredo reviewed Oct 23, 2024
View reviewed changes
geekosaur
geekosaur reviewed Oct 23, 2024
View reviewed changes
@philderbeast philderbeast force-pushed the doc/install-fourmolu-10472 branch 2 times, most recently from 4e5842a to 6662af4 Compare October 23, 2024 22:35
@philderbeast philderbeast force-pushed the doc/install-fourmolu-10472 branch from f248783 to 9ed9d76 Compare October 25, 2024 13:37
@philderbeast
Copy link
Collaborator Author

I've squashed commits as much as I can without dealing with merge conflicts (brought on by reordering). Wanted also to keep commits from reviewers (their suggested changes) separate from my own.

@philderbeast
Copy link
Collaborator Author

Label merge+no rebase is necessary when the pull request is from an organisation.

@mergify mergify bot added the ready and waiting Mergify is waiting out the cooldown period label Oct 25, 2024
@ulysses4ever
Copy link
Collaborator

Sorry for the bother but I'd expect a PR titled "Add a note for contributors installing fourmolu" to have one commit. Can we squash everything?

@ulysses4ever ulysses4ever self-requested a review October 25, 2024 14:15
@ulysses4ever ulysses4ever removed merge+no rebase ready and waiting Mergify is waiting out the cooldown period labels Oct 25, 2024
ulysses4ever
ulysses4ever requested changes Oct 25, 2024
View reviewed changes
Copy link
Collaborator

@ulysses4ever ulysses4ever left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, squash.

@philderbeast
Copy link
Collaborator Author

I hadn't wanted to squash everything down to one, to stomp on the contributions of others, but am happy to do that now that I've got the go-ahead.

@philderbeast @geekosaur @ulysses4ever
Add a note for contributors installing fourmolu
328318f 
- Code review suggested changes
  - Redundant "that".
  - Use indefinite article.
  - Assume ghc version installed via ghcup
  - Use `--with-compiler` option
  - Grammar, "its requirement"
- Move --with-compiler to a tip
- Use --numeric-version
- Use hyphen in series-specific
- Capitalise GHC
- Note restriction on system ghc version.
- Add a warning about switching fourmolu versions

Co-Authored-By: brandon s allbery kf8nh <[email protected]>
Co-Authored-By: Artem Pelenitsyn <[email protected]>
@philderbeast philderbeast force-pushed the doc/install-fourmolu-10472 branch from 9ed9d76 to 328318f Compare October 25, 2024 15:40
@geekosaur
Copy link
Collaborator

If they're resolved it's not really a problem. And my own observation suggests GitHub keeps the old commits around (maybe from the reflog?) so the comments are still available.

@ulysses4ever
Copy link
Collaborator

There's also an option to manually add co-authors to the final commit, if you feel like it:

$ git commit -m "Refactor usability tests.
>
>
Co-authored-by: NAME <[email protected]>
Co-authored-by: ANOTHER-NAME <[email protected]>"

https://docs.github.com/en/pull-requests/committing-changes-to-your-project/creating-and-editing-commits/creating-a-commit-with-multiple-authors

@ulysses4ever
Copy link
Collaborator

Oh, you did it already, cool...

@mergify mergify bot added ready and waiting Mergify is waiting out the cooldown period merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days labels Oct 25, 2024
@mergify mergify bot merged commit 63c486a into haskell:master Oct 27, 2024
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jasagredo jasagredo jasagredo left review comments

@geekosaur geekosaur geekosaur approved these changes

@ulysses4ever ulysses4ever ulysses4ever approved these changes

Assignees
No one assigned
Labels
merge delay passed Applied (usually by Mergify) when PR approved and received no updates for 2 days merge+no rebase ready and waiting Mergify is waiting out the cooldown period
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Note which GHC versions contributors can use for installing fourmolu
4 participants
@philderbeast @ulysses4ever @geekosaur @jasagredo