Using rustfmt systemically

Development
21

Yeah, I am using nightly by default. This is why I want to add a rustfmt.toml file (and the call to try the config was for everyone).

I don’t think there is a pre-push hook just yet, and but they are pretty easy to add.

22

Could we use latest rustmft in stable? Not everybody has nightly, expecially an up-to-date nightly.

23

I use nightly sometimes, but I always compile Pijul with the latest stable (NixOS rocks!). I think sticking to stable is really important.

1 Like
24

I just noticed that if we run a beautifier in a pre-commit hook when using git the changes will not be added to the commit itself—they’ll remain unstaged in the working dir. This seems to be intentional, as it’s considered dangerous. It’s preferred to hook the beautifier to the editor (run on save).

Maybe we should adopt this change too? that is:

  1. change our hook to just check whether the code is beautiful or not, refusing to commit if it isn’t.
  2. prevent pijul to include in the commit the changes (if any) done in pre-commit hooks
1 Like
25

You are totally right.

I am struggling a bit with contributing with pijul and writing my PhD thesis currently, but I will try to have a look this week-end to propose a standard hook (and to update the changelog notes).

2 Likes
26

I have started messing around with rustfmt. My current opinion: we should use cargo +nightly fmt, as long as cargo fmt won’t use rustfmt-1.0.

27

Looking at the state of rustfmt, it seems we would need to improve our nix files a bit before we can consistently use it (Rust doesn’t need Rustup on NixOS, yet can use the binaries downloaded by Rustup).

28

I will leave that to you, as I am not a NixOS user myself.

29

Ok, it seems the stable rustup channel includes rustfmt-1.0 now, so I’m doing this today, and add a hook to my repository. I should add a way on the Nest to reject badly-formatted patches.

30

I reported this on IRC, but do it here just in case other folks have run in the same problems as I did.

It looks like your patch introduced conflicts on pijul upstream :\ I guess this is related to the fix your patch introduces. Unrecording your two lasts patches put pijul in a state where I cannot compile it anymore (rustc complains about invocation and sign not being found).

Hope this help.

Edit: Okay. So it looks like, cloning the repository where I have unrecorded your last two patches allows me to build Pijul again. Either your fix in rust fmt patch fixes the issue, or an issue remains in pijul record.
On a related note, I think it could be a good idea to record your fix in a dedicated patch, rather than mixing it with the formatting thing. Of course, I don’t know if this is feasible (that’s the nasty thing on bootstrapping a VCS)

One a side note, if you want some help to avoid this kind of situation, I would be more than willing to help by doing review and testing on your patches prior to their merge in Pijul master :).