Opened 5 years ago

Closed 2 years ago

#10954 closed bug (fixed)

Install and begin using Gerrit

Reported by: kallisti5 Owned by: haiku-web
Priority: normal Milestone: Unscheduled
Component: Website Version: R1/Development
Keywords: gerrit Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

We need to install and begin using gerrit to track patches towards releases. This is currently a blocker towards R1A5.

Change History (9)

comment:1 by kallisti5, 5 years ago

Milestone: R1/alpha5
Version: R1/Development

comment:2 by nielx, 5 years ago

I will be able to help you out here but I would really like to see more discussion on the impact of our workflow.

comment:3 by kallisti5, 5 years ago

The overall goal is to utilize gerrit to filter commits adginst a release branch.

For Example:

  • Haiku R1A5 is branched
  • Developer thinks a change should go into R1A5
    • Developer pushes change to mainline, and the R1A5 branch
    • R1A5 commit is queued for peer / RC review and accepted / discussed / rejected

This is nice because it makes the person pushing the change, and who knows the code best, decide something should make it into a release. The other devs and RC can then give feedback.

How it has worked in previous releases was the release coordinator cherry picked stuff, and generally too much was pulled in.

in reply to:  3 comment:4 by luroh, 5 years ago

Replying to kallisti5:

The overall goal is to utilize gerrit to filter commits against a release branch.

This sounds more like a means of reaching some goal than a goal in itself.

For Example:

  • Haiku R1A5 is branched
  • Developer thinks a change should go into R1A5
    • Developer pushes change to mainline, and the R1A5 branch
    • R1A5 commit is queued for peer / RC review and accepted / discussed / rejected

This approach would suffer from the same problem as during the previous cycles, people running and testing fixes on the master branch instead of the release branch. Fixes should be developed on the release branch and merged to master where appropriate.

This is nice because it makes the person pushing the change, and who knows the code best, decide something should make it into a release. The other devs and RC can then give feedback.

How is that? It sounds more like this supposedly knowledgeable person is pushing the commit into master, while it gets queued up for peer review for the release branch. Granted, it would save the RM some work keeping track of '+alpha' comments in the commit logs. It however wouldn't solve the problem of people forgetting to nominate their commits for branch inclusion.

comment:5 by luroh, 5 years ago

To note, I am not against the idea of using Gerrit in general. For example, Gerrit might be of interest if it can be used as part of a gatekeeping solution to help us prevent build breakage.

comment:6 by pulkomandy, 5 years ago

There are several use cases for Gerrit:

  • Experimenting with the release branch to avoid manual cherry picking
  • Changing our patch submission workflow so patches go on Gerrit instead of Trac. This immediately adds the "apply patch" button everyone has been requesting, and also adds a better interface for inline code review. With further configuration, this can be paired with the build bots so each patch is ran through them, and ideally, we would also run a style checker. As a result, developers need to review a patch only for functionality and design, rather than spending time on checking that it builds and follows the coding guidelines. Patch submitter gets immediate feedback on wether his patch has guidelines violations or breaks the build.
  • I also think moving our after-the-fact code reviews on the commits ML to Gerrit might be a good idea, there were concerns that we don't have the manpower for systematic code review and that it would slow down development ; but I will certainly run my patches through it so I'm not the guy who breaks the build and introduces style violations anymore.

This also opens the way for more patch validation, eg running our UnitTests on the patches. I hope we can get a more complete set of unit tests to make this really useful.

comment:7 by waddlesplash, 5 years ago

Priority: blockernormal

Yeah, I don't think this is a blocker for the next release. I can live without it.

Also, Luroh's suggestion to just deploy fixes to the "release" branch and merge to master sounds like a good idea to me, if the other devs agree (and remember) to do so.

comment:8 by waddlesplash, 5 years ago

Milestone: R1/alpha5Unscheduled

comment:9 by waddlesplash, 2 years ago

Resolution: fixed
Status: newclosed

Gerrit is now operational.

Note: See TracTickets for help on using tickets.