Discourse patch submission is not fit for purpose

Development
#13

One thing that seems fairly obvious to me but maybe should be stated
is that the person who acts as a proxy also becomes the owner of those
changes. Somebody who submits patches by email cannot become an owner
as they will not get the notifications from GitHub or participate in
the ecosystem meaningfully (eg: no RFC contributions). In that sense,
this list should be transparent to the normal workflow. It’s an
unofficial add-on that caters to the needs of certain contributors.

That’s true, although the person who wants something from the original
submitter could probably just email them themselves if they wanted to
(and get a faster response).

They wouldn’t have to, though.

#14

It’s not just about testing. It’s the whole pull request life cycle that would have to be adapted for email.
It’s not an insignificant amount of work if you aim for the same quality as we do in Github pull requests.

I think it’s a fundamentally bad idea to have multiple contribution and review flows, having this will create a second class of contributors who cannot participate fully in the community.

#15

The same, as in «and make sure it is no better»? In email I get all notifications but the patch context GitHub attaches is often insufficient; in the Web UI finding out all that has happenned since some time is a lot of work (and the full context might have been destroyed). Reasonable people manually replying to emails with patches usually end up with easier to read discussion…

2 Likes
#16

I think it’s a fundamentally bad idea to have multiple contribution
and review flows, having this will create a second class of
contributors who cannot participate fully in the community.

We already have this, and always have – I’m not trying to introduce
it, I’m trying to improve what we have. And the alternative is a second
class of potential contributors who are locked out of contributing at
all.

#17

I am just wondering maybe is worth considering RIOT as a platform ?

Nixos-rebuild failed (pango-compat bug)
#18

I am just wondering maybe is worth considering RIOT as a platform ?

I don’t think so. Riot is a chat app. We’re looking for something for
long-form patch submission and code review. The options discussed so
far are already widely used for this, are integrated into Git, etc.

1 Like
#19

I’d be happy to review patches from mail, and my personal patch submission flow already aims to let me ignore GitHub’s PR submission flow in favour of one that lets me pretend I’m dealing with mail, without making that fact particularly visible to others.

Since GitHub regularly destroys patch history, I already use tools meant for the email flow like git-series in order to track the history of my PRs.

I’m not too convinced of the value of sourcehut, especially when public-inbox and regular mailing lists already exist, provide cloneable archives and highlighted patches, and are already in use by the kernel and many adjacent communities.

1 Like
#20

@edef are you okay having your email address published publicly?

Either create a mailing list or give me your email and I will transform the patches section of discourse with a message redirecting the user to it.

#21

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 :

#22

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/

#23

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

#24

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
#25

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.

#26

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
#27

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
#28

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
#29

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
#30

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
#31

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
Contribute without a github account?
#32

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.

Hosted by Flying Circus.