Discourse patch submission is not fit for purpose

Sorry for necrobumping the topic but the email workflow for submitting patches still pops up here and there and we never really solved it (unless I missed something), for instance today a potential patch appeared on reddit :
https://www.reddit.com/r/NixOS/comments/flwrlh/seaweedfs/

Yeah I think we need some way to handle that. Personally I do not mind
receiving patches via Email. Most of my daily workflow evolves around
email. Such as reading these threads that are just mails in my inbox.
Just like another other mailinglist or GitHub Repo that I am following.

A few of my friends do not ever want to have a GitHub account
(especially after the recent acquisitions) and thus rather submit
throught me or via some ML.

It probably goes a bit too far for our topic here but there is also
discussions around how to modernize the entire mail based workflow
without having a vendor lock-in over on the public-inbox ML [1].
Unfortunately, that discussion didnā€™t go far.

I was also thinking about hosting a patchwork [2][3] instance for this
kind of activity. While I can certainly do some work around accepting
patches (and translating them into PRS & commits) I wouldnā€™t start this
without a few more comitters that are willing to help out.

The workflow around that kind of ML archive (unless you have all the
mail locally) could look similar to this:

# create a new branch
nixpkgs/ $ git checkout -b some-patch origin/master
# apply the patch (series)
nixpkgs/ $ git am < $(curl https://something/patch/123/mbox/)
# review & test
nixpkgs/ $ git log / tig / nix-build ā€¦

This really isnā€™t that much different from checking out a PR (minus
being on the right commit when you apply the series).

Afterwards you can just open a PR or commit as usual or simply reply to
those patches via Email if you want to request changes / provide
feedback.

The big problem here is probably not sovling this (patch receiving &
applying) technically but how to handle feedback and CI results in those
cases. As we are pretty much stuck on GH right now. The troubles here
could be that you first argue with the submitter via E-Mail, do multiple
iterations of patches until everyone is happt. Afterwards you put that
PR up on GH and then some more feedback arrives. It would probably not
be great if those that accept that patches then have the burden of
synchronzing between GH and responding to feedback. GH doesnā€™t even
allow unregistered users to subscribe to updates on a PR/issue.

On the other hand, if the patches we are receiving via Email are usually
small, self-contained and do not cause long dicussions then I donā€™t see
why this wouldnā€™t work.

[1] RFC: Using public-inbox v2 repos for distributed patch lifecycle tracking - Konstantin Ryabitsev
[2] patchwork
[3] https://lore.kernel.org/patchwork/project/lkml/list/

There are 5 patches in Patches - NixOS Discourse, I recommend someone handles that and we optimize once the pain kicks in.

IMHO the pain already kicks in since discourse isnā€™t a proper mailing
list. It mangles with all the inputs.

ā†’ missing attachment, not really actionable. Copying a patch from the
modified mail output is also not fun.

ā†’ the pastebin that was used to circument discourse shortcomings
expired

ā†’ has been handled by ryantm

ā†’ more corrupt patches due to discourse

Of course we can now try to repair all those issues and still call it
not painful. At least in my case those patches also arrived via email
but were converted into some markdown gibberish. Not very helpful.

Patch submission to our project should just work like everywhere else
(where mail is supported): git-send-email shouldnā€™t be rejected as
invalid and should not be mangled by the ML software (discourse in our
case).

2 Likes

In that case we can create a team of a few volunteers that are willing to review these patches and document whom to send the email to. For 5 patches thereā€™s really no need to go further than that.

Just as a brief update on the matter. I started receiving a few mails with patches over the last year and eventually (a as of a few days ago) started pointing those to ~andir/nixpkgs-dev archives ā€” lists.sr.ht. It is by no means official and I just try to work on the on a best-effort basis.

4 Likes

I started sending patches to besaid mailing list and the overall experience is really nice so far:

No signup required anywhere, @andir and others promptly replied with valuable feedback and so far have even merged some of the patches.

It enables users like me to submit ā€“ I dare say ā€“ valuable improvements, which would otherwise remain as local diffs due to Nix(OS)'s currently exclusive development community.

3 Likes

Were these patches merged without a PR on github? It is unpleasant that a few people continue to commit directly into master with any form of PR. Iā€™m going to find working with nixpkgs more difficult if we add more sources of changes coming in that Iā€™m not following.

ping @andir for clarification of the process.

5 Likes

I agree that we should have one canonical review flow and while I do not like GitHub PRs that is what we have and will probably for the forseeable future.

I did push a few very low-profile contritubtions directly but most of them were just being proxied into PRs.

As creating PR for each contribution is mondane and annoying I started to write some basic tooling to make the process smoother, safer and to always create a PR for every contribution that comes in via email. Thus It is less tempting to just push it to get the work done:

https://asciinema.org/a/i2Apv7QYrd9382nJfizHtXRM0

I also add a reference to the PR (and a message-id to the commits) in case there was discussions before the contribution was put into a PR (commit style, eval errors, ā€¦).

Now that I have some basic automation I am happy to add more in case there are still doubts about these contributions coming in as PRs from me.

5 Likes

That is great @andir! I think this creates a more welcoming community while keeping things manageable. I appreciate the work you have done on this :heart:

1 Like

a quick update on nixpkgs-dev. we have a handful of people submitting and reviewing patches there now. we have some tooling hosted here: README - nixpkgs-ml-tools - nixpkgs mailing list tools which weā€™ve been using to collect patch review before submitting to github. Iā€™m currently using the gh tool to submit pull requests, and email to respond to review, so no need to ever touch the github web interface which is nice.

for anyone looking to contribute via email this is the best place right now. join us! hopefully we can make this a little more official in the future once more people join.

3 Likes

Has anyone considered writing a discourse plugin to handle patches? Iā€™m envisaging something that would either try to detect if a message contains patches, or would simply be active for any thread in the patches category. A nice rendering of the patch (at the very least, just displaying it as plain text), with a link to download the original message (Could it add the unmangled email as an attachment?).

Iā€™m not familiar with ruby or with discourse plugins, so take this suggestion with a pinch of salt.