Update CONTRIBUTING.md #455
Conversation
philberty
requested review from
NalaGinrut,
tschwinge,
dkm,
flip1995,
thomasyonug,
SimplyTheOther,
yizhepku and
CohenArthur
CONTRIBUTING.md
Outdated
| @@ -29,6 +29,8 @@ | |||
| * The PR policy: Everything has to go through a PR | |||
| - An exception to this rule will be the merge commits of updating the repo against upstream GCC | |||
|
|
|||
| * Patches sent via email are welcome, these will be subject to the same Copyright Assignment standards. Each patch will be formed into a GitHub PR to follow the normal bos process. | |||
There was a problem hiding this comment.
boRs.
Maybe you should clarify where the review is happening in this case: review on the ml, and the PR is created when it has been accepted so not a real review and sent directly for merge?
There was a problem hiding this comment.
Good point. A problem with accepting patches from the mailing list is when a patch might need changes, and requires us to update the PR.
I think doing code review on GitHub is much more powerful than via mail so we could still do the review here on GitHub via the PR. So when we say yes thanks for your patches and create the PR, we should forward the link back to the submitter to say here is your code under review. If the PR needs changes we can apply the patch updates from the mailing list but please expect some delays in this process due to the manual step involved.
"Reviews for the patches will be carried out via the PR on GitHub, this will be a publicly viewable resource. The link will be forwarded back to the submitter, as a way of viewing the state of their changes and as a way to manage any review comments. If a PR needs changes, please expect some delays in the management of updating the PR, due to the manual process involved."
There was a problem hiding this comment.
Crazy idea: What about creating a bot that automatically creates PR's based on patches on the mailing list and updates them as new patches are submitted to the mail thread? It could also forward comments both directions, so eg if you mail quoting parts of the patch, a github review gets created for the relevant lines and as you reply to them on github a mail gets sent to the mailing list. (will probably need some buffering to avoid spam on the mailing list)
There was a problem hiding this comment.
git does something like this with gitgitgadget. Probably overkill for this project, since git is mainly developed on the mailing list, not on GitHub, but stuff like this already exists.
There was a problem hiding this comment.
b4's implementation may also prove helpful to automate stuff around patches from mailing lists.
There was a problem hiding this comment.
Oh, I see GCC is already there : https://patchwork.ozlabs.org/project/gcc/list/
There was a problem hiding this comment.
I really like the idea of automating this, is there a way for b4 to actually have a github user to create PRs? Or did I just miss that part.
There was a problem hiding this comment.
AFAIK, b4 is primarily meant as a command-line tool for maintainers. I mentioned it because you may be to reuse part of its implementation to write the bot that would then contact GitHub etc.
Related: in Rust for Linux we are also trying to adapt the kernel development workflow to GitHub :-) Our bot is @ksquirrel. I do not plan on automating the mailing list side until we are in mainline though (and even then, it will depend on the amount of patches from the mailing list).
There was a problem hiding this comment.
What is @ksquirrel used for ? Was looking at existing solution for better integration of ml patch and github, and only find ways to create mail from PR, not the other way round.
There was a problem hiding this comment.
The goal is to have @ksquirrel automate many things of the kernel development workflow and allow us to use GitHub. Ideally, it will do all this:
- Review commit messages for common errors (check for signatures, formatting and common practices, detect common errors such as lkml links, etc.) -- this is already implemented and I keep adding checks when I discover something new that it can check. See example at Add SPI Abstraction Rust-for-Linux/linux#264 (expand the hidden comments).
- Pick up
Acked-by,Reviewed-by,Tested-bytags from reviewers and then appending them at merge time etc. - Run the CI, for every commit, just before the merge.
- Collect patches from the ML and put them in GitHub automatically.
I do not think we will need the opposite direction (PR to mail), but anyway, it does not sound like a hard thing to implement. I would just do it from scratch if you only need that.
|
I streamlined the text a bit; good to go, as far as I'm concerned. |
We have been accepting patches via mail from developers following the normal GCC processes. Let's update our guidelines to reflect this. Co-authored-by: CohenArthur <[email protected]> Co-authored-by: Thomas Schwinge <[email protected]> Co-authored-by: Miguel Ojeda <[email protected]>
|
bors r+ |
|
Build succeeded: |
We have been accepting patches via mail from developers following the normal GCC processes. Let's update our guidelines to reflect this.