Skip to content

Remove nightly toolchain flag from recommended precommit#1220

Merged
spacebear21 merged 1 commit intopayjoin:masterfrom
mehmetefeumit:contribution-doc-update
Dec 12, 2025
Merged

Remove nightly toolchain flag from recommended precommit#1220
spacebear21 merged 1 commit intopayjoin:masterfrom
mehmetefeumit:contribution-doc-update

Conversation

@mehmetefeumit
Copy link
Copy Markdown
Contributor

@mehmetefeumit mehmetefeumit commented Dec 10, 2025

I asked Claude to give me bash code to select between nix and non-nix environments. So this PR in its entirety was written with AI... UNTIL there was an edit based on spacebear's recommendation below, and the change turned from adding a branching logic to account for the nix shell, to just removing the toolchain flag.

Pull Request Checklist

Please confirm the following before requesting review:

Originally, this PR was for adding an if-statement to the recommended pre-commit hook so that it does not attempt to find +nightly for rust formatter. However, as spacebear mentions below, we do not need it anymore as it is enforced. So this change simply removes that flag now.

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented Dec 10, 2025

Pull Request Test Coverage Report for Build 20121565157

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 83.176%

Totals Coverage Status
Change from base Build 20109603088: 0.0%
Covered Lines: 9665
Relevant Lines: 11620

💛 - Coveralls

@benalleng
Copy link
Copy Markdown
Collaborator

I am hesitant to have two diverging paths for the pre-commit hook. perhaps now with the ohttp-relay in the monorepo we can just encourage the use of the nix dev env such that the pre-commit must be done within the nix shell?

@spacebear21
Copy link
Copy Markdown
Collaborator

I don't think we actually need +nightly in the pre-commit hook anymore, since the toolchain is now enforced in rust-toolchain.toml.

@mehmetefeumit mehmetefeumit force-pushed the contribution-doc-update branch from 011acc9 to aa787f1 Compare December 11, 2025 04:04
@mehmetefeumit mehmetefeumit changed the title Add nix branch for fmt in the recommended precommit hook Remove nightly toolchain flag from recommended precommit Dec 11, 2025
Copy link
Copy Markdown
Collaborator

@benalleng benalleng left a comment

Choose a reason for hiding this comment

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

Ack aa787f1 It would be nice in a followup to address the test step now requiring you to first enter the nix dev shell with the ohttp-relay integration tests.

Copy link
Copy Markdown
Contributor

@nothingmuch nothingmuch left a comment

Choose a reason for hiding this comment

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

utACK

@spacebear21 spacebear21 merged commit 91db248 into payjoin:master Dec 12, 2025
10 checks passed
@mehmetefeumit mehmetefeumit deleted the contribution-doc-update branch December 14, 2025 23:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants