Skip to content
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

genemichaels: apply patch to only write files if changed #365303

Conversation

djacu
Copy link
Member

@djacu djacu commented Dec 15, 2024

When formatting rust files with genemichaels, the timestamp of the file is always modified whether or not the file was changed. This is problematic for detecting whether a file has been reformatted or not, such as when using tools such as pre-commit, treefmt, and treefmt-nix.

See the following:
andrewbaxter/genemichaels#95
andrewbaxter/genemichaels#96
numtide/treefmt-nix#277

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
  • Tested basic functionality of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

Add a 👍 reaction to pull requests you find important.

When formatting rust files with `genemichaels`, the timestamp of the
file is always modified whether or not the file was changed. This is
problematic for detecting whether a file has been reformatted or not,
such as when using tools such as `pre-commit`, `treefmt`, and
`treefmt-nix`.

See the following:
andrewbaxter/genemichaels#95
andrewbaxter/genemichaels#96
numtide/treefmt-nix#277
@djacu
Copy link
Member Author

djacu commented Dec 15, 2024

Result of nixpkgs-review pr 365303 run on x86_64-linux 1

1 package built:
  • genemichaels

@ofborg ofborg bot added 11.by: package-maintainer This PR was created by the maintainer of the package it changes 10.rebuild-darwin: 1 10.rebuild-linux: 1 labels Dec 15, 2024
@jmbaur
Copy link
Contributor

jmbaur commented Dec 16, 2024

Looks like the patch was merged and included in the latest tag! Can we bump that in this PR?

# bumped to a release that has this change
# See: https://github.com/andrewbaxter/genemichaels/issues/95
# See: https://github.com/andrewbaxter/genemichaels/pull/96
./genemichaels-only-write-if-changed.patch
Copy link
Member

Choose a reason for hiding this comment

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

It'd be better to fetch the patch from GitHub rather than vendoring it in-tree

Suggested change
./genemichaels-only-write-if-changed.patch
(fetchpatch {
url = "https://github.com/andrewbaxter/genemichaels/commit/3c58f090888053b5165dbb0292b5974293851e2a.patch";
hash = "sha256-vxtO9N6P6rcfGhIfagr2fT83o0lM6J1tOckxarMJkL8=";
})

@getchoo
Copy link
Member

getchoo commented Dec 17, 2024

Looks like the patch was merged and included in the latest tag!

Or update, oops!

@djacu
Copy link
Member Author

djacu commented Dec 17, 2024

I didn't expect them to merge my PR so quickly. I'll make a new PR to bump the version and close this.

@djacu
Copy link
Member Author

djacu commented Dec 17, 2024

Closing in favor of #365966

@djacu djacu closed this Dec 17, 2024
@djacu djacu deleted the djacu/patch-genemichaels-only-write-on-change branch December 17, 2024 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants