Skip to content

workflows/check-nix-format: Improve error message#337577

Merged
infinisil merged 1 commit intoNixOS:masterfrom
tweag:improve-check-nix-format-message
Dec 4, 2024
Merged

workflows/check-nix-format: Improve error message#337577
infinisil merged 1 commit intoNixOS:masterfrom
tweag:improve-check-nix-format-message

Conversation

@infinisil
Copy link
Member

Description of changes

Looks like the error message could be a bit clearer still: #337109 (comment) 😄

Ping @NixOS/nix-formatting


Add a 👍 reaction to pull requests you find important.

Looks like the error message could be a bit clearer still: NixOS#337109 (comment)
@github-actions github-actions bot added the 6.topic: policy discussion Discuss policies to work in and around Nixpkgs label Aug 26, 2024
@infinisil infinisil mentioned this pull request Aug 26, 2024
13 tasks
@ofborg ofborg bot added 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. labels Aug 26, 2024
@Redhawk18
Copy link
Member

Who is @NixOS/nix-formatting? It should highlight to a github account.

@infinisil
Copy link
Member Author

infinisil commented Aug 27, 2024

@Redhawk18 Not sure what you mean. That's a GitHub team, comprised of the people on the Nix Formatting Team, and it's highlighted for me

@dasJ
Copy link
Member

dasJ commented Aug 27, 2024

@infinisil only for org members iirc :/

@eclairevoyant
Copy link
Contributor

eclairevoyant commented Aug 27, 2024

There's one other thing I wanted to mention but kept forgetting about:
If ci/pinned-nixpkgs.json differs between the last commit on the base branch and the base commit of the PR, we may want to suggest that the contributor fetch the base branch and rebase their PR on it

Copy link
Contributor

@MattSturgeon MattSturgeon left a comment

Choose a reason for hiding this comment

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

I agree this is clearer, +1 from me 🙂

if (( "${#unformattedFiles[@]}" > 0 )); then
echo "Some new/changed Nix files are not properly formatted"
echo "Please go to the Nixpkgs root directory, run \`nix-shell\`, then:"
echo "Please format them using the Nixpkgs-specific \`nixfmt\` by going to the Nixpkgs root directory, running \`nix-shell\`, then:"
Copy link
Contributor

@MattSturgeon MattSturgeon Aug 31, 2024

Choose a reason for hiding this comment

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

If this were single-quoted, I think the backticks wouldn't need escaping anymore?

Suggested change
echo "Please format them using the Nixpkgs-specific \`nixfmt\` by going to the Nixpkgs root directory, running \`nix-shell\`, then:"
echo 'Please format them using the Nixpkgs-specific `nixfmt` by going to the Nixpkgs root directory, running `nix-shell`, then:'

Not important though, if you'd rather consistently use double-quotes.

@wegank wegank added the 12.approvals: 1 This PR was reviewed and approved by one person. label Sep 4, 2024
@MattSturgeon MattSturgeon added 12.approvals: 2 This PR was reviewed and approved by two persons. and removed 12.approvals: 1 This PR was reviewed and approved by one person. labels Dec 4, 2024
@infinisil infinisil merged commit fab3778 into NixOS:master Dec 4, 2024
@infinisil infinisil deleted the improve-check-nix-format-message branch December 4, 2024 19:46
@infinisil
Copy link
Member Author

Let's aim for more optimistic merging :D

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

Labels

6.topic: policy discussion Discuss policies to work in and around Nixpkgs 10.rebuild-darwin: 0 This PR does not cause any packages to rebuild on Darwin. 10.rebuild-linux: 0 This PR does not cause any packages to rebuild on Linux. 12.approvals: 2 This PR was reviewed and approved by two persons.

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

8 participants