Idea: rustfmtbot

nikomatsakis · 2017-04-04T13:25:17.179Z

So I was thinking about the problem of how to ensure that all PRs are rustfmt’d for some of my repos. It occurred to me that the simplest thing would be to have some kind of GitHub hook that automatically runs rustfmt. Ideally, this would be something that one can very, very easily integrate into your repo in one of two modes:

Run rustfmt on every PR automatically when it is opened
- if any changes occur, push a commit to the user’s branch, and leave a comment
Run rustfmt on PRs when requested (e.g., @rustfmtbot format)
- if any changes occur, push a commit to the user’s branch, and leave a comment

This all (hopefully) works because GH now allows you to push to other people’s branches, if you own the repo on which they have opened a PR. I would love to write this but I don’t know much about how GH works and I don’t really have the time. Anybody interested? Any reasons this can’t work?

cc @nrc

steveklabnik · 2017-04-04T14:31:15.000Z

The only thing they’d need is to add the bot as a collaborator, then this would totally work.

Introducing Sedy, the Serverless GitHub Bot That Fixes Typos for you

To facilitate typo fixes and simple code reviews, Sedy brings the power of the sed Linux command to Github comments.

which is similar and very interesting.

seanmonstar · 2017-04-04T17:41:03.878Z

Can’t you include in your CI to run rustfmt --diff, and error if there is a diff?

cuviper · 2017-04-04T17:42:38.103Z

nikomatsakis:

This all (hopefully) works because GH now allows you to push to other people’s branches, if you own the repo on which they have opened a PR.

This is optional by the submitter, “allow edits from maintainers”, but I think it is enabled by default.

nikomatsakis · 2017-04-04T19:54:08.775Z

seanmonstar:

Can’t you include in your CI to run rustfmt --diff, and error if there is a diff?

You could, but that’s annoying – it forces the person opening the PR to fix it (or me), and so forth. Then I have to wait for them to respond etc. I’d prefer to just make it automatic.

nikomatsakis · 2017-04-04T19:54:31.953Z

cuviper:

This is optional by the submitter, “allow edits from maintainers”, but I think it is enabled by default.

Yes. Seems fine, everybody leaves it on in practice.

withoutboats · 2017-04-04T22:17:58.346Z

It could also be a step of a bors merge, no? I guess you want the diff to follow the formatting as well.

nikomatsakis · 2017-04-05T01:33:38.770Z

Indeed, but I want this to be usable on any project, not just those that use bors. But I guess if we made bors mega easy to install that might be ok too. (In my ideal world, doing cargo new would give you whatever files you need to have everything be setup already (along with a suitable travis configuration).)

nrc · 2017-04-09T21:49:33.380Z

This sounds like it would be a fun project - there is no reason why it shouldn’t work - both the GH APIs and Rustfmt have everything you’d need (although the feature for formatting a selection of text in Rustfmt is a bit flakey, so this would probably find a bunch of bugs).

I’m not sure how easy it is to get a set of ranges of modified lines from a PR. At worst you could process the diff which wouldn’t be too bad, I assume there are libs to help with that. (Because Rustfmt can’t format a diff, you’d need to format the selections in the PR’s branch).

Nemo157 · 2017-04-10T07:33:24.398Z

nrc:

I’m not sure how easy it is to get a set of ranges of modified lines from a PR

For a first implementation it seems like it could just run rustfmt over the whole codebase, if you’re using this bot then it’s likely that you’re keeping the entire codebase formatted correctly, so the only parts that might need reformatting are the files modified in the PR.

killercup · 2018-10-31T21:48:19.207Z

RIght now, rustfmtbot could probably be implemented as a regular Github user that just happens to write a lot of reviews with applicable suggestions (and that gives up when there are too many issues)!

notriddle · 2018-10-31T22:23:09.784Z

Too bad suggestions have to be exactly one line.

felix91gr · 2018-11-01T04:34:39.872Z

Yeah, but they’re in beta and receiving feedback. I’d expect them to add support for multi-line suggestions soon, it just makes sense.

torkleyy · 2018-11-01T07:26:52.983Z

We’ve had a similar issue with Amethyst. The fact that rustfmt changes / changed its formatting very often made it hard to keep the crate formatted.

We thought about a bot, but that would cause a lot of rebasing, which just becomes too much work for larger PRs.

Currently, we are using https://concourse-ci.org/ to show separate status reports (one for the build, one for the book tests and one for rustfmt).

Screenshot%20from%202018-11-01%2008-24-18-crop

Screenshot from 2018-11-01 08-24-18-crop.png965×401 34.6 KB

That way, we can ask contributors to format whenever it makes sense. Note that the status report shown here does not make a merge with bors fail, so it’s just indicative.

nikomatsakis · 2019-03-25T08:28:18.044Z

This topic was automatically closed 90 days after the last reply. New replies are no longer allowed.