Discourse patch submission is not fit for purpose

Development
1

In the old days, before my time, rather than Discourse, there was a
mailing list called nix-dev. Sometimes people sent patches to the
mailing list.

When we moved to Discourse, the Patches category was created to
facilitate email patch submission. Recently, I’ve seen a number of
patches submitted this way, and that’s led me to the conclusion that the
way things currently are in this area isn’t fair to potential
contributors.

The way patches are generally sent over email is using git send-email.
This creates a message where the Subject is the first line of the commit
message, and then the rest of the commit message and the diff comprise
the message body. Reviewers can then reply, and use the quoted original
message inserted by their mail client to provide inline patch review.
Then, once the patch is ready to be merged, a committer can use git am
to apply to patch and push it.

This works very well with normal email, or over a mailing list, but
Discourse breaks it. When it receives a patch email, it will try to
interpret it as Markdown (or something), and this leads to the patch
being mangled to the point where it will likely be unreadable, and
almost certainly no longer apply. The alternative is to create a patch
file with git format-patch and attach it to an email, where it will be
turned into a Discourse attachment. Discourse won’t include the
attachment in notification emails it sends out, though, so to actually
review the patch, you have to go to Discourse in a web browser, download
the attachment, open it in a text editor, and copy the contents back
into Discourse or your mail client. Discourse will still try to
interpret the quoted code in your reply as Markdown, making it very
difficult to follow what the conversation actually is.

The huge headaches caused by Discourse here mean that patches submitted
this way are rarely reviewed, and almost never end up actually being
applied to Nixpkgs. I don’t think that this is because people aren’t
interested in reviewing them – I’ve seen other people try, and I’d be
extremely willing to, if Discourse didn’t make it so extremely
difficult. The fact that despite this, we get patches submitted this
way anyway proves that there is definitely demand from contributors for
this. But I don’t think it’s fair to potential contributors to have an
officially-sanctioned contribution mechanism that will almost never
result in their work being acknoweledged and reviewed.

So, I think we have two options here:

  1. Insist that potential contributions only come through GitHub.
  2. Have a mailing list (only) for patch submission.

I would be strongly opposed to 1., for several reasons. People clearly
want to be able to send patches over email, as the use of the category
shows. I don’t think people should have to sign a EULA to be able to
contribute to Nixpkgs. And, it’s much easier to explain the patch
workflow to a newbie than GitHub pull requests. It’s “clone, change,
stage, commit, git send-email (type in your smtp credentials)” compared
to “clone, change, create and check out branch, stage, commit, fork, add
remote, push, create pull request”, including several context switches
between terminal and browser.

I’d like to here @zimbatm’s thoughts especially, as the person I think
was largely behind the move to Discourse, and who wrote the description
we have for the Patches category1. Overall, the move to Discourse has
been fantastic for our community, but I think it would be a shame if
this had to fall by the wayside because of it – I don’t think there’s
any reason we can’t have Discourse for community discussions, and
working patch submission over email.

14 Likes
2

Hi @qyliss,

I agree with your assessment that Discourse is unfortunately not adequate to handle patches submitted by email. I assumed that it would handle attachments properly which it doesn’t. One reason for using the hosted discourse was that my time is fairly limited, which also shows with the half-assed replacement.

One of the most credible alternative for email-based developer that I have seen is https://sourcehut.org. It would be nice if somebody were to investigate how to mirror nixpkgs over there. And maybe even forward notifications to this forum. Is that something you would be interested in pursuing?

6 Likes
3

Wow, sourcehut sounds really interesting!

Another possibility for email-based workflow is that people could volunteer to accept patch emails on others’ behalf. For instance, anyone is welcome to email me and I’ll forward it along. (I’m chreekat everywhere—my address and gpg key aren’t hard to find.)

1 Like
4

I have used sr.ht since it went into public alpha, almost a year ago. And it is really nice. They also have NixOS CI images and I have found builds to be far more reliable than e.g. Travis CI (I don’t think a build ever failed due to CI errors). sr.ht also has this feature where you can ssh into the VM for a certain time frame after the build fails to do manual inspection.

It seems that Drew DeVault is really open to cooperating with other open source projects, so it’s definitely worth a try to see if nixpkgs could be mirrored. Though I guess that for an e-mail patch submission workflow, it would be enough to start a mailing list on https://lists.sr.ht/ . E.g. see the sr.ht development list as an example:

https://lists.sr.ht/~sircmpwn/sr.ht-dev

1 Like
5

Drew is awesome. FYI @andir had a go at importing nixpkgs in sr.ht and failed, it seems like we are exercising the platform’s scalability due to the 200k commit that nixpkgs has. Drew is aware of it and is working on a fit. To be continued.

4 Likes
6

We don’t need to mirror the Nixpkgs git repo, right? We just need a
mailing list.

I have a mild (because it’s only a mailing list, and can’t be that
difficult to get right) preference for not using SourceHut. In my
evaluation, I have found it to not work particularly well (not
unreasonably, given it’s in alpha), but what I’ve found more concerning
is the attitude of the author towards bug reports. For example, here he
tells me that it is my problem that his software doesn’t support the
common practice of posting revisions of a patchset in reply to the
first: https://todo.sr.ht/~sircmpwn/lists.sr.ht/128

That’s not to say we couldn’t mirror a mailing list to SourceHut –
that’s one of the nice things about mail, and I gather Drew is working
on support for it.

One option: I already host mailing lists for Spectrum (see
https://spectrum-os.org/participating.html#mailing-lists), and am
committed to and funded for doing so for at least the next year. I
would be happy to add an additional list for Nixpkgs. It could be
mirrorred and easily replaced should I suddenly disappear or whatever,
and should SourceHut look more convincing in future it would be easy to
migrate. Available on that server are Mailman, public-inbox (for
archiving and web browsing), and Hyperkitty (a nice modern web UI
suitable for people who don’t want to use mail).

I don’t think we should try to mirror mailing list discussion to
Discourse either – it would end up being ugly on both sides, I think.
It might be better to create a simple web application that would show
proposed changes from both the mailing list and GitHub, that reviewers
could follow.

1 Like
7

@qyliss looking at the bug report, it seems like something got lost in translation. Drew is developing a whole new developer platform, and handling all the customer support himself. It’s possible he might have rushed and not properly understood what you meant exactly. The whole platform is based on the idea of patch-based development so it would be surprising if common scenarios weren’t something that was intended to be handled.

I don’t have a strong preference either way. Whatever solution is selected in the end, I think the important criteria is that there are some people with nixpkgs push rights monitoring and handling the submissions. The disadvantage of both options right now is that they don’t integrate with the existing ecosystem and are yet another platform that needs an account.

8

I don’t have a strong preference either way. Whatever solution is
selected in the end, I think the important criteria is that there are
some people with nixpkgs push rights monitoring and handling the
submissions. The disadvantage of both options right now is that they
don’t integrate with the existing ecosystem and are yet another
platform that needs an account.

I don’t think that either would need an account – usually you can
subscribe to a mailing list just by sending an email, and with mine at
least you don’t even need to be subscribed to post.

But, yes, I agree with the second point. So, Nixpkgs committers, please
reply to this post if you would be willing to monitor such a list,
provide review, and push contributions to Nixpkgs.

9

I would totally do that!

1 Like
10

I don’t think we should even accept patches over email.

Patches over email doesn’t fit our current workflow at all and it requires someone else to take your patch and create a Github pull request for the patch to enter our normal review and test flow.

While I understand and agree that Github has it’s set of problems having a completely separate review flow without any testing infra is something I’m strongly opposed to.
Maintaining nixpkgs is already hard enough.

I think it’s time to formally say patches over email is not an acceptable form of contribution for Nixpkgs.

2 Likes
11

So testing is your only concern? If so, it’s solvable.

12

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.

As long as there are people willing to do the work, I don’t see much harm in having this. That’s also the beauty of the nix community, we have a good diversity in types of users.

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 ?

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.