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

Idea: bot that pretty prints on commit #238

Closed
vjeux opened this issue Jan 16, 2017 · 10 comments
Closed

Idea: bot that pretty prints on commit #238

vjeux opened this issue Jan 16, 2017 · 10 comments
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken

Comments

@vjeux
Copy link
Contributor

vjeux commented Jan 16, 2017

I was thinking in the context of a github project, how to make sure that the files stay pretty printed.

The most obvious way is to add a CI rule that fails if the file is not pretty printed, but in practice that's super annoying as you only get the feedback way later and don't even get an email notification that it failed.

Instead, I was thinking of letting maintainers merge non pretty printed files and then have a CI hook that pretty prints all the changed files. If there's a change, then have a bot commit the pretty printed version to master directly.

Because github doesn't check the author field, we can even have the commit under the original author name and link to the actual commit to make sure that the blame and attribution is clean.

I'd love thoughts on this, is this crazy?

@hzoo
Copy link
Contributor

hzoo commented Jan 16, 2017

Not crazy, was thinking about a bot for this for eslint --fix for a long time. For our newly created https://github.com/babel/babel-bot we want to comment with the log failure and I thought if we could check the logs (not sure the most cost effective) to see if a failure was a linting failure then can make a command to autofix). Yeah could make a commit and if the diff is good keep it in the PR.

@bnjmnt4n
Copy link
Contributor

This is a pretty common idea actually. Various projects make use of their CI for a build step in order to ensure builds are updated properly. For example, the HTML5 Boilerplate has a Travis script that will build it after every change. In fact, @alrra has made a few easy to use scripts to allow branches to be updated (if necessary) from Travis: https://github.com/alrra/travis-scripts.

@jlongster
Copy link
Member

Great minds! I think there are three workflows people can choose from:

  1. Use lint-staged to run the formatter check as a precommit hook and will fail if anything isn't formatted (the user then runs a command to format all of them). lint-staged is a lot better than running the formatter across an entire project because it'll be a lot faster: just runs it on changed files. (disclaimer: I haven't looked at that project yet but it seems promising)
  2. Have CI check if entire project is formatted and fail if not
  3. Do what you described above, and "autofix" it afterwards!

I think #3 is a really cool idea. We basically need a "last resort" option to make sure everything is formatted. Ideally, a github bot could automatically check PRs and if it detects something isn't formatted it could offer to do a follow-up commit or something like that. I like PRs being completely finished before actually pulling the merge button, and that would give you the chance to squash the format PR in with the others. Can github bots add commits to PRs?

@vjeux vjeux added the status:needs discussion Issues needing discussion and a decision to be made before action can be taken label Jan 16, 2017
@hzoo
Copy link
Contributor

hzoo commented Jan 16, 2017

Can github bots add commits to PRs?

Since maintainers can edit other people's PRs now via https://github.com/blog/2247-improving-collaboration-with-forks

then I think it should be possible if the bot is a collaborator?

@suancarloj
Copy link

Could it not be just a simple precommit hook that check the staged files ?

@vjeux
Copy link
Contributor Author

vjeux commented Jan 16, 2017

While you could do some pre-commit hook or changing the pull request, it's likely going to be annoying as you are going to change someone else branch, generate merge conflicts...

I think the safest approach is to look at master and append a commit that pretty prints whatever files were touched.

@clarkbw
Copy link

clarkbw commented Jan 18, 2017

Can github bots add commits to PRs?

We are using pushing changes from CircleCI back to GreenKeeper PRs because we are using yarn and want the yarn.lock file updated. See firefox-devtools/debugger#1740 You need to give a r/w key to the CI to allow for the CI to push changes back to the repo. Assuming people always pull down changes before continuing work on PRs this could be an option for you. Here's the script that runs in CircleCI after successful tests.

@kassens
Copy link
Contributor

kassens commented Apr 10, 2017

I found this but haven't tried it myself yet: https://github.com/GordyD/prettier-master

@vjeux
Copy link
Contributor Author

vjeux commented Apr 10, 2017

@GordyD came up with the idea, he started it before going to parental leave. Not sure what's the current status is.

@vjeux
Copy link
Contributor Author

vjeux commented Jul 6, 2017

Let's close this as we're not going to actively work on it, I still believe it would be great to have though!

@vjeux vjeux closed this as completed Jul 6, 2017
@lock lock bot added the locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. label Jul 7, 2018
@lock lock bot locked as resolved and limited conversation to collaborators Jul 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
locked-due-to-inactivity Please open a new issue and fill out the template instead of commenting. status:needs discussion Issues needing discussion and a decision to be made before action can be taken
Projects
None yet
Development

No branches or pull requests

7 participants