Hi Jakub, On Fri, Sep 29, 2023 at 03:17:23PM +0200, Jakub Czapiga wrote:

...

Clearly I must be doing something wrong as the patches have been gathering dust for around three years now.

There might be few reasons for that. First, for patch to be eligible for review it cannot have merge conflicts as resolving them might change patch contents and will most certainly drop all CR+ scores and invalidate some comments. You should try keep patches in mergable state.

My patches all depend on one another, Gerrit seems to treat this as "conflict" when this is entirely normal so I don't see how I can do what you suggest. I (used to) frequently rebase against master to make sure actual conflicts don't stay there for long FYI. I push these using HEAD:refs/for/main as was documented last time I checked (3y ago) not sure what else is expected of me.

Second, submitting long patch trains usually makes reviewers to halt until previous patches are reviewed, as changes in patches earlier in the train might invalidate their time put into reviewing. I'd suggest focusing on earlier patches first, merging them and gradually moving to later ones.

Another proplem here being that gerrit makes it very hard to figure out what the "first" patch is anyway. They get shuffled into a random order every time I push the series. Am I as the submitter supposed to draw the reviewer's attention to which is the first patch or what? That can't possibly be right. The gerrit interface sure makes it seem like reviewers are notified of the fact that they're supposed to do something. UI lies I suppose. Honestly this gerrit thing may work OK for companies where people are properly incentivized to do their assigned reviews or people in the in-group that know who to poke to get things moving, but for outside contributors the experience is even more fundamentally broken than the email/ML patch workflow and that's saying something. Consider this as a point of project self-interest. I would have probably been busy writing/reviewing coreboot patches for the past three years if my contributions didn't end up getting dropped on the cherry-pickin room floor. As I see it: I could have just bought a 50c Winbond flash chip and called it a day, yet here I am refactoring code, adding vendor support and writing tests for security critical code. Isn't that exactly the type of highly motivated individual contributor the coreboot project should be trying to attract? Well your infrastructure/processes are getting in the way of that. Please do better <3 --Daniel

Hi Daniel,   Let me start this overly large email by saying that I appreciate you saying something. We don't get enough feedback about issues and what we need to improve. That said, not getting reviewers on all patches is unfortunately somewhat of a known issue that we're actively working to correct, though we could definitely use help in that regard. Here are some thoughts: # New Contributor Hashtag One thing we're doing now that we didn't do three years ago is identifying new users so that we can pay attention to helping people who aren't as familiar with the community. You can see those patches here: https://review.coreboot.org/q/repo:coreboot+status:open+hashtag:new_contribu... # Greeting new users I'm also sending a greeting to new users (people with fewer than 5 commits), which unfortunately wouldn't have helped your series of SPI patches. Here's a sample greeting: ``` Hi, It looks like you haven't contributed to the coreboot project before. Welcome, and thank you for the patch. We hope that this is just the first of many. Please let us know if there's anything we can do to help get your first sets patches merged as you get used to the contribution process. The coreboot project has a hands-off policy regarding other people's patches so nobody here is going to update the contents without your permission. If you would you like someone to take over your patches at any point, please just post a comment to that effect on the specific patch. You might find the coding style guide and the gerrit guidelines useful to read. https://doc.coreboot.org/contributing/coding_style.html https://doc.coreboot.org/contributing/gerrit_guidelines.html If you want to talk with anyone, you can talk to developers on one of the many options we have: https://doc.coreboot.org/community/forums.html Again, please let us know if you have any questions, or if there's anything we can do to help. ``` If you have any thoughts on how to improve the greeting, I'd appreciate any feedback. # Search for non-corporate users We also have a list of corporate users so that we can search for non-corporate users. This is to help people who don't have coworkers backing them up to ask for reviews. https://review.coreboot.org/q/repo:coreboot+status:open+-ownerin:corporate # Maintainers list As mentioned previously, we have a list of maintainers for various areas. Unfortunately, we don't have a maintainer for the SPI code, so we don't have anyone who has said they'll take responsible for reviewing a number of your patches. Additionally, the people who are on the maintainers list frequently maintain a LOT of things simply because we don't have enough really experienced people to review everything that gets pushed. I'm honestly overwhelmed by the number of reviews I get added to. I think I have over 300 in my queue right now. # Gerrit as an issue You mention that you see gerrit as a problem, but don't think the issue is gerrit. My guess is that you're simply not familiar enough with it to use the features it offers properly - This isn't meant to be a slight towards you, there is definitely a learning curve. If you click on any of your patches, you should see a section called "Relationship Chain" generally in the upper middle right of the page. That's the patch train, in order. The first page that you see is simply ordered as the list of patches most recently touched in some fashion. It's not meant to show any sort of relationship. # We need ideas on *how* to improve It's fine to say that the coreboot project should "do better", and you're not wrong, but without something concrete that we can actually do, that's not something we can just agree to do. The coreboot project does not have any paid employees to do the work, and without getting significantly more contributions to support someone, I don't see that happening anytime soon. There are a number of companies supporting work for coreboot, but they're all supporting engineers responsible for their particular sections. Any work done in other areas for the community good, is generally done by volunteers. Asking unpaid volunteers to be strictly responsible for things is difficult. This is a problem that many open source projects have. # coreboot Leadership Meeting I'd really like to ask you to attend the leadership meeting - we hold it every two weeks. You can find the time and how to join on the coreboot calendar: https://coreboot.org/calendar.html Thanks again for saying something about the problem. We do want to improve and make things better for everyone. Martin Sep 29, 2023, 07:53 by dxld@darkboxed.org:

Hi Jakub,

On Fri, Sep 29, 2023 at 03:17:23PM +0200, Jakub Czapiga wrote:

...

...

Clearly I must be doing something wrong as the patches have been gathering dust for around three years now.

There might be few reasons for that. First, for patch to be eligible for review it cannot have merge conflicts as resolving them might change patch contents and will most certainly drop all CR+ scores and invalidate some comments. You should try keep patches in mergable state.

My patches all depend on one another, Gerrit seems to treat this as "conflict" when this is entirely normal so I don't see how I can do what you suggest.

I (used to) frequently rebase against master to make sure actual conflicts don't stay there for long FYI.

I push these using HEAD:refs/for/main as was documented last time I checked (3y ago) not sure what else is expected of me.

...

Second, submitting long patch trains usually makes reviewers to halt until previous patches are reviewed, as changes in patches earlier in the train might invalidate their time put into reviewing. I'd suggest focusing on earlier patches first, merging them and gradually moving to later ones.

Another proplem here being that gerrit makes it very hard to figure out what the "first" patch is anyway. They get shuffled into a random order every time I push the series.

Am I as the submitter supposed to draw the reviewer's attention to which is the first patch or what? That can't possibly be right. The gerrit interface sure makes it seem like reviewers are notified of the fact that they're supposed to do something. UI lies I suppose.

Honestly this gerrit thing may work OK for companies where people are properly incentivized to do their assigned reviews or people in the in-group that know who to poke to get things moving, but for outside contributors the experience is even more fundamentally broken than the email/ML patch workflow and that's saying something.

Consider this as a point of project self-interest. I would have probably been busy writing/reviewing coreboot patches for the past three years if my contributions didn't end up getting dropped on the cherry-pickin room floor.

As I see it: I could have just bought a 50c Winbond flash chip and called it a day, yet here I am refactoring code, adding vendor support and writing tests for security critical code. Isn't that exactly the type of highly motivated individual contributor the coreboot project should be trying to attract?

Well your infrastructure/processes are getting in the way of that.

Please do better <3

--Daniel _______________________________________________ coreboot mailing list -- coreboot@coreboot.org To unsubscribe send an email to coreboot-leave@coreboot.org

Hi Daniel, On Fri, Sep 29, 2023 at 6:53 AM Daniel Gröber <dxld@darkboxed.org> wrote:

Honestly this gerrit thing may work OK for companies where people are properly incentivized to do their assigned reviews or people in the in-group that know who to poke to get things moving, but for outside contributors the experience is even more fundamentally broken than the email/ML patch workflow and that's saying something.

I am curious how is gerrit review worflow is more broken than the emai/ML patch worflow, i.e. what do you think is better in the latter? -vb

Hi Daniel, I admittedly am very loosely familiar with the email based reveiw flow, my experience if from many years back, and I don't have fond memories of it :) On Thu, Oct 5, 2023 at 10:28 AM Daniel Gröber <dxld@darkboxed.org> wrote:

...

The email-based git workflow (which GH copied fairly well) does things right here by having the "default" be for people to get notified of all project activity and sending reviews being as easy as an email reply.

Isn't triggering an email as easy in gerrit? All you need to do is to add reviewer(s) in the 'Reply' popup and click 'send'. Or is your concern that the email is not sent to the wide audience by default? Another is this: Something I do when I get to know a project is subscribe

to the ML or equivalent and passively watch patches, reviews and banter to get a feel for how things are done here. I have no idea how you would do this with gerrit other than by actively adding yourself to CC on patches (which I feel is too visible to really be an equivalent) or actively deathscrolling through the patch lists, which I'm (personally) not going to remember to do periodically.

Just in case this is not known: in gerrit you can subscribe to receive emails about all patches sent to a git repo: you click on the cog in the top right, then on 'Notifications' on the next screen, and then you can configure what emails on what repositories you want to receive. Would this address your concern? Cheers, -vb

Hi Vadim, Patrick, On Thu, Oct 05, 2023 at 11:18:19AM -0700, Vadim Bendebury wrote:

I admittedly am very loosely familiar with the email based reveiw flow, my experience if from many years back, and I don't have fond memories of it :)

IMO sr.ht has innovated quite nicely here. Their ML manager (lists.sr.ht) detects patches and shows things like status APPLIED/PENDING/NACKed (based on git activity IIRC) and CI build status. Removing some of the legacy objections to the git email workflow. (Side note: The APIs are all open and built to be modular so jenkins integration would likely not be hard) Anyone can reply from the webinterface through their email client with a mailto:// link or The "Forward this thread to me" button, which hast to be the best ML invention since sliced bread eer I mean threads. Notice: no account necessary. You just go ahead and do that thing you want to do. You can prepare/send patchsets from the webinterface (with an account) to lower the entry barrier for not-yet git-email afficionados. Until git(ea) PR federation through activitypub comes to fruition and is widely adopted I'm putting my decentralized eggs into the git-email basket. That being said I'm not sure this particular tool is right for coreboot. I'm just saying in some aspects (barrier to contribute, community building) this is perhaps the gold standard now.

Just in case this is not known: in gerrit you can subscribe to receive emails about all patches sent to a git repo: you click on the cog in the top right, then on 'Notifications' on the next screen, and then you can configure what emails on what repositories you want to receive. Would this address your concern?

I didn't know that but IMO what gerrit sends out is absolutely not equivalent to a normal patch ML.

There's also the coreboot-gerrit mailing list (archive, although it doesn't thread emails properly, at https://mail.coreboot.org/hyperkitty/list/coreboot-gerrit@coreboot.org/lates...) that can be subscribed to.

We used to have these emails end up on the main mailing list (for familiarity after we moved from a mailing list + svn based flow to Gerrit + git in 2011), but people complained that there's too much noise on the list.

I mean, look at the emails gerrit sends they're almost pure spam, IMO that's another dimension of the problem with gerrit. I'm overwhelemed enought by the gerrit emails I get from just my own patches. When humans send patches and humans send replies to those patches we don't get useless and unactionable mails like "attention is required by" and "comments were posted" without the actual comments. I might want to read those comments, Gerrit, grrrr! I think this problem is multiplied because in gerrit everything is per-patch rather than per-series. I've never had this much of a problem with large series in gitlab even though it can be chatty too. Something I've seen, but that may not work for coreboot is to keep gerrit for project members/teams working closeley together, but allow [PATCH] submission on the ML for outsiders. idk. just a baseline for discussion. --Daniel