Are stacked diffs/PRs supported for external contributors?

[cphlipot1]

I have read https://perfetto.dev/docs/contributing/developer-tools#-quot-stacked-diffs-quot-with-github and it appears these tools work by setting the base branch for the stacked PR to that of the base PR. This presents an issue for stacking since external contributors do development using forks, so the base branches of stacked PRs would appear on the forked repo instead of the official perfetto repo.

I'm aware of other methods of tracking PR dependencies like overlapping PRs where the base branch is set to "main" for all PRs which is used by https://sapling-scm.com/docs/addons/reviewstack and plays nicely with stacked PRs from forks. This however generally requires the reviewers to switch to an alternate review tool (like reviewstack) that can understand these PR relationships. I think those approaches may work for external contributors, but I'm not sure that is something reviewers would want to deal with.

Are the stacked diff tools meant to be used by external contributors? if yes, how? if not, are there any alternative suggestions?

[LalitMaganti]

So the answer is no and we feel this is a very unfortunate limitation. It's we tired really hard to come up with when we first moved to Github to workaround but we didn't find anything we were happy with.

I'm aware of other methods of tracking PR dependencies like overlapping PRs where the base branch is set to "main" for all PRs which is used by https://sapling-scm.com/docs/addons/reviewstack and plays nicely with stacked PRs from forks.

I checked out sapling but I didn't feel like https://reviewstack.dev/ was up to scratch. I just opened it again and it froze my browser tab :/

I also tried Graphite but was again unimpressed by the constant push to pay (I would have liked to try out full feature without paying) and also found it quite buggy.

Are the stacked diff tools meant to be used by external contributors?

So the tools as they exist in the repo only work for internal contributors I'm afraid.

if not, are there any alternative suggestions?

Not that we've found. Very open to suggestions. One thing we did consider: making dev/ open to trusted contributors who have a proven history of sending CLs to Perfetto. You would definitely meet that bar. @primiano what do you think?

[cphlipot1]

So the answer is no and we feel this is a very unfortunate limitation.

Thanks for clarifying. I think this might be worth mentioning as a small disclaimer just to make that a bit more clear in https://perfetto.dev/docs/contributing/developer-tools#-quot-stacked-diffs-quot-with-github

I checked out sapling but I didn't feel like https://reviewstack.dev/ was up to scratch. I just opened it again and it froze my browser tab :/

I have not observed any issues in the limited amount of time i've used it, but that doesn't mean the tool does not have issues. Is the main issue the stability, or is it the design/workflow you take issue with?

Not that we've found. Very open to suggestions

i have not found any perfect solutions either. Most of these tools feel like workarounds for missing github features, and IMO a proper solution would need to come via Github itself in how it can represent PRs.

One thing we did consider: making dev/ open to trusted contributors who have a proven history of sending CLs to Perfetto. You would definitely meet that bar. @primiano what do you think?

While a more generally applicable solution would be best, I think this is probably the most feasible solution for my immediate issues if you are ok with doing this.

[LalitMaganti]

I have not observed any issues in the limited amount of time i've used it, but that doesn't mean the tool does not have issues. Is the main issue the stability, or is it the design/workflow you take issue with?

Stability is the biggest problem if it's a tool we use day-in day out. But the home page design is not exactly something I'm a fan of either.

i have not found any perfect solutions either. Most of these tools feel like workarounds for missing github features, and IMO a proper solution would need to come via Github itself in how it can represent PRs.

Yeah agreed it should be a Github improvement

[primiano]

Two comments:

1) On the topic of tooling,

TBH I am still very unhappy about how stacked changes work even if you have dev/ write access. That's the one place where I'm really missing Gerrit. I think the only answer is that one day when i'll have free time I'll (ask AI to) write me a frontend for github PR that looks as identical as possible to gerrit. Until then we'll have to live with what we have unforutnately.

I liked graphite.dev in principle, but it fails every single time I try to add a comment. Maybe it has to do with the subscription state... dunno

I haven't tried reviewstack much but doesn't seem too mature.

I'd also be open to try out other workflows as I'm still not sold on the current one we have for stacked changes.

Maybe we could try something like:

• Make all the stacked changes in a PR, one commit per change
• When one is portion of the stack is ready to go, create a copy in a dedicate PR and land it, and rebase the main PR? This would work only if we can come up with some github action that automates the part "take the LGTM-ed commits and land them separately"

Proposal here are welcome

2) On the topic of giving non-googlers dev/ write access

I think it's a pragmatic choice and I'd be in favour of it % figuring out a set of rules we consistently stick to.
In this specific case, @cphlipot1 is a no-brainer due to the track record of contributions. There is no discussion needed.

However, if we give @cphlipot1 write access, tomorrow somebody else will ask "how can I get write access too?" and we must have an answer. The answer cannot be "depends on how primiano@ (or somebody else) feels about it".

My proposal here would be something along the lines of Chromium's committership, which I'd summarize as follows:

• Somebody nominates a person for getting write access
• The person being nominated must have a history of at least 10 non-trivial good quality patches
• Two other committers have to support the nomination
• If no controversy, the person is given write access within X days (7?)

If we like the idea, pragmatically we need:

• A .md document that explains the process
• A CONTRIBUTORS or similar file with the github handles. I want the source of truth to live somewhere other than github settings, as it is too easy to accidentally add/remove people there.

Thoughts?

[LalitMaganti]

Thoughts?

Agree with copying the Chromium model. It's battle tested and if it works for Chromium, it should work for us.

[primiano]

Having said this: #2291 just landed

[primiano]

Aaand #2297