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

pre-commit hooks on merge #161

Open
Eh2406 opened this Issue Apr 9, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@Eh2406
Copy link

Eh2406 commented Apr 9, 2018

Cargo just stopped using rustfmt in travis, it was just too annoying to keep up with. I was thinking of building a bot that submitted PR's when cargo +stable fmt had changes. But we already have a bot, can homu just run fmt and amend its merge commits?

@nox

This comment has been minimized.

Copy link
Member

nox commented Apr 9, 2018

Seems more like a job for highfive, if it needs to happen on push.

@nox nox closed this Apr 9, 2018

@nox

This comment has been minimized.

Copy link
Member

nox commented Apr 9, 2018

Oops, didn't mean to close, sorry.

@nox nox reopened this Apr 9, 2018

@Eh2406

This comment has been minimized.

Copy link

Eh2406 commented Apr 9, 2018

The goal is:

  • master is always using cargo fmt
  • no human ever needs to run that command to get CI to pass.

Well the workflow I imagine is:

Alis is new to the project and rust tooling.
She clones the master branch of the project.
She makes some changes. She accidentally gets rls to format it. The master branch of the project uses fmt so only her changes get formatted.
Next in the revue some changes are requested.
She makes the changes over the course of several tentative commits. None of those CI runs are wasted on "CI is red because you did not run fmt." No one needs to explain what fmt is nore how to use it.
When the reviewer is satisfied homu merges the PR, to ensure the master branch of the project uses fmt homu runs cargo fmt on the merge before committing/testing/pushing.

Also I don't think highfive is set up to make commits.

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