Re: automated coding style tool
Joel Brobecker via Gdb <[email protected]>
| Newsgroups | gmane.comp.gdb.devel |
|---|---|
| Message-ID | <Z4pzhRX/[email protected]> |
> I don't think adding this tool is going to be the magic bullet everyone > seems to think it is. > > As a reviewer you'll either still need to learn the style in order to > spot when a contributor has not running the formatting tool. Either > that, or you need to apply each patch locally and the check run the > formatting tool to check the formatting is correct. I have experience with a project where developers were asked to install a pre-commit check that would run the formatting tool, but where we did not have (yet) a pipeline to verify the formatting at submission time. This project had a lot of developers, some of them very occasional. It's true that, at first, we had a number developers forgot to install that pre-commit hook, and ended up submitting ill formatted code. What I noticed, at least for myself, is that with time you start developing an eye for what the expected formatting is like. And when I noticed something odd, I'd just ask if they installed the pre-commit and ran the formatting tool. If they say no, I pointed them to the directions, and asked them to retest and resubmit. Ultimately, we mostly converged. And for the cases we didn't notice at review - no big issue. The formatting shouldn't be horrible of the reviewer would have noticed it, so no great harm -- certainly no worse than right now. And later on, the next developer touching the same file ends up noticing that the formatting seems wrong. It's a bit of a pain for them to first run the formatting tool and push a pure-reformatting commit. But we tell the contributor who forgot, they install the tool, and normally we eventually converge. Another things we can imagine doing is a nightly job on sourceware that runs the nightly snapshot through the formatting tool, and sends an email with a list of files that are not conformant. Where I think it might be painful is which version of the tool to use. Different versions will likely have slightly different results in terms of formatting the code. You might want to require a specific version and check for that version, to avoid different users having different ways of formatting the same code. > I think if GDB could just move away from the mailing list and each users > pushes their own patches model, over to a pull request style approach, > then we could potentially have _real_ automated checks, e.g. checking > the formatting. Then we have an _actual_ win. Seconded. In a model like this, you don't have to ask. You run a job that checks it, and you know if it's conformant or not. -- Joel