Thread

1. cfbot update: Using GitHub for patch review

   Jelte Fennema <postgres@jeltef.nl> — 2024-06-21T14:36:13Z

   I recently got write access to the cfbot repo[1] and machine from
   Thomas. And I deployed a few improvements this week. The most
   significant one is that it is now much easier to use GitHub as part of
   your patch review workflow.
   
   On the cfbot website[2] there's now a "D" (diff) link next to each
   commit fest entry.  A good example of such a link would be the one for
   my most recent commitfest entry[3]. There is a separate commit for
   each patch file and those commits contain the "git format-patch"
   metadata. (this is not done using git am, but using git mailinfo +
   patch + sed, because git am is horrible at resolving conflicts)
   
   The killer feature (imho) of GitHub diffs over looking at patch files:
   You can press the "Expand up"/"Expand down" buttons on the left of the
   diff to see some extra context that the patch file doesn't contain.
   
   You can also add the cfbot repo as a remote to your local git
   repository. That way you don't have to manually download patches and
   apply them to your local checkout anymore:
   
   # Add the remote
   git remote add -f cfbot https://github.com/postgresql-cfbot/postgresql.git
   # make future git pulls much quicker (optional)
   git maintenance start
   # check out a commitfest entry
   git checkout cf/5065
   
   P.S. Suggestions for further improvements are definitely appreciated.
   We're currently already working on better integration between the
   commitfest app website and the cfbot website.
   
   P.P.S The "D" links don't work for patches that need to be rebased
   since before I deployed this change, but that problem should fix
   itself with time.
   
   [1]: https://github.com/macdice/cfbot
   [2]: http://cfbot.cputube.org/
   [3]: https://github.com/postgresql-cfbot/postgresql/compare/cf/5065~1...cf/5065
   
   
   
   

2. Re: cfbot update: Using GitHub for patch review

   Nathan Bossart <nathandbossart@gmail.com> — 2024-06-21T15:54:50Z

   On Fri, Jun 21, 2024 at 04:36:13PM +0200, Jelte Fennema-Nio wrote:
   > I recently got write access to the cfbot repo[1] and machine from
   > Thomas. And I deployed a few improvements this week. The most
   > significant one is that it is now much easier to use GitHub as part of
   > your patch review workflow.
   
   Nice!  Thank you.
   
   -- 
   nathan
   
   
   
   

3. Re: cfbot update: Using GitHub for patch review

   Josef Šimánek <josef.simanek@gmail.com> — 2024-06-21T15:56:06Z

   pá 21. 6. 2024 v 16:36 odesílatel Jelte Fennema-Nio <postgres@jeltef.nl> napsal:
   >
   > I recently got write access to the cfbot repo[1] and machine from
   > Thomas. And I deployed a few improvements this week. The most
   > significant one is that it is now much easier to use GitHub as part of
   > your patch review workflow.
   >
   > On the cfbot website[2] there's now a "D" (diff) link next to each
   > commit fest entry. A good example of such a link would be the one for
   > my most recent commitfest entry[3]. There is a separate commit for
   > each patch file and those commits contain the "git format-patch"
   > metadata. (this is not done using git am, but using git mailinfo +
   > patch + sed, because git am is horrible at resolving conflicts)
   
   This is brilliant!
   
   > The killer feature (imho) of GitHub diffs over looking at patch files:
   > You can press the "Expand up"/"Expand down" buttons on the left of the
   > diff to see some extra context that the patch file doesn't contain.
   >
   > You can also add the cfbot repo as a remote to your local git
   > repository. That way you don't have to manually download patches and
   > apply them to your local checkout anymore:
   >
   > # Add the remote
   > git remote add -f cfbot https://github.com/postgresql-cfbot/postgresql.git
   > # make future git pulls much quicker (optional)
   > git maintenance start
   > # check out a commitfest entry
   > git checkout cf/5065
   >
   > P.S. Suggestions for further improvements are definitely appreciated.
   > We're currently already working on better integration between the
   > commitfest app website and the cfbot website.
   >
   > P.P.S The "D" links don't work for patches that need to be rebased
   > since before I deployed this change, but that problem should fix
   > itself with time.
   >
   > [1]: https://github.com/macdice/cfbot
   > [2]: http://cfbot.cputube.org/
   > [3]: https://github.com/postgresql-cfbot/postgresql/compare/cf/5065~1...cf/5065
   >
   >
   
   
   
   

4. Re: cfbot update: Using GitHub for patch review

   Pavel Stehule <pavel.stehule@gmail.com> — 2024-06-21T16:09:42Z

   pá 21. 6. 2024 v 17:55 odesílatel Nathan Bossart <nathandbossart@gmail.com>
   napsal:
   
   > On Fri, Jun 21, 2024 at 04:36:13PM +0200, Jelte Fennema-Nio wrote:
   > > I recently got write access to the cfbot repo[1] and machine from
   > > Thomas. And I deployed a few improvements this week. The most
   > > significant one is that it is now much easier to use GitHub as part of
   > > your patch review workflow.
   >
   > Nice!  Thank you.
   
   
   +1
   
   good work
   
   Pavel
   
   >
   >
   > --
   > nathan
   >
   >
   >
   

5. Re: cfbot update: Using GitHub for patch review

   Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2024-06-28T13:10:04Z

   On Fri, Jun 21, 2024 at 8:06 PM Jelte Fennema-Nio <postgres@jeltef.nl>
   wrote:
   
   > I recently got write access to the cfbot repo[1] and machine from
   > Thomas. And I deployed a few improvements this week. The most
   > significant one is that it is now much easier to use GitHub as part of
   > your patch review workflow.
   >
   > On the cfbot website[2] there's now a "D" (diff) link next to each
   > commit fest entry.  A good example of such a link would be the one for
   > my most recent commitfest entry[3]. There is a separate commit for
   > each patch file and those commits contain the "git format-patch"
   > metadata. (this is not done using git am, but using git mailinfo +
   > patch + sed, because git am is horrible at resolving conflicts)
   >
   > The killer feature (imho) of GitHub diffs over looking at patch files:
   > You can press the "Expand up"/"Expand down" buttons on the left of the
   > diff to see some extra context that the patch file doesn't contain.
   >
   > You can also add the cfbot repo as a remote to your local git
   > repository. That way you don't have to manually download patches and
   > apply them to your local checkout anymore:
   >
   > # Add the remote
   > git remote add -f cfbot https://github.com/postgresql-cfbot/postgresql.git
   > # make future git pulls much quicker (optional)
   > git maintenance start
   > # check out a commitfest entry
   > git checkout cf/5065
   >
   > P.S. Suggestions for further improvements are definitely appreciated.
   > We're currently already working on better integration between the
   > commitfest app website and the cfbot website.
   >
   > P.P.S The "D" links don't work for patches that need to be rebased
   > since before I deployed this change, but that problem should fix
   > itself with time.
   >
   
   Thanks. Very helpful.
   
   Will it be possible to make it send an email containing the review
   comments? Better even if a reply to that email adds comments/responses back
   to PR.
   
   I need to sign in to github to add my review comments. So those who do not
   have a github account can not use it for review. But I don't think that can
   be fixed. We need a way to know who left review comments.
   
   There was some discussion at pgconf.dev about using gitlab instead of
   github. How easy is it to use gitlab if we decide to go that way?
   
   -- 
   Best Wishes,
   Ashutosh Bapat
   

6. Re: cfbot update: Using GitHub for patch review

   Thomas Munro <thomas.munro@gmail.com> — 2024-06-28T23:12:56Z

   On Sat, Jun 29, 2024 at 1:10 AM Ashutosh Bapat
   <ashutosh.bapat.oss@gmail.com> wrote:
   > I need to sign in to github to add my review comments. So those who do not have a github account can not use it for review. But I don't think that can be fixed. We need a way to know who left review comments.
   
   I don't think Jelte was talking about moving review discussion to
   Github, just providing a link to *view* the patches there.  Now I'm
   wondering if there is a way to disable comments on commits in the
   postgresql-cfbot GH account.  I guess they'd be lost after 48 hours
   anyway when the branch gets force-pushed and commit hash changes?  I
   don't want people to start posting comments there that no one is
   looking at.
   
   > There was some discussion at pgconf.dev about using gitlab instead of github. How easy is it to use gitlab if we decide to go that way?
   
   cfbot could certainly be configured to push (ie mirror) the same
   branches to gitlab too (I don't have much experience with Gitlab, but
   if it's just a matter of registering an account + ssh key, adding it
   as a remote and pushing...).  Then there could be [View on Github]
   [View on Gitlab] buttons, if people think that's useful (note "View",
   not "Review"!).  The Cirrus CI system is currently only capable of
   testing stuff pushed to Github, though, so cfbot would continue to
   push stuff there.
   
   If memory servers, Cirrus used to say that they were planning to add
   support for testing code in public Gitlab next, but now their FAQ says
   their next public git host will be Bit Bucket:
   https://cirrus-ci.org/faq/#only-github-support
   
   Given that cfbot is currently only using Github because we have to to
   reach Cirrus CI, not because we actually want Github features like
   issue tracking or pull requests with review discussion, it hardly
   matters if it's Github, Gitlab or any other public git host.  And if
   we eventually decide to move our whole workflow to one of those
   systems and shut down the CF app, then cfbot will be retired, and
   you'll just create PRs on that system.  But so far, we continue to
   prefer the CF app + email.
   
   The reason we liked Cirrus so much despite the existence of many other
   CI systems including the ones build into GH, GL, etc and many 3rd
   party ones, was because it was the only provider that allowed enough
   compute minutes for our needs, supported lots of operating systems,
   and had public links to log files suitable for sharing on out mailing
   list or cfbot's web interface (if you click to see the log, it doesn't
   say "Rol up roll up, welcome to Foo Corporation, get your tickets
   here!").  I still don't know of any other CI system that would be as
   good for us, other than building our own.  I would say it's been a
   very good choice so far.  The original cfbot goal was "feed the
   mailing list to a CI system", with Github just a necessary part of the
   plumbing.  It is a nice way to view patches though.
   
   
   
   

7. Re: cfbot update: Using GitHub for patch review

   Jelte Fennema <postgres@jeltef.nl> — 2024-06-29T08:42:23Z

   On Sat, 29 Jun 2024 at 01:13, Thomas Munro <thomas.munro@gmail.com> wrote:
   >
   > On Sat, Jun 29, 2024 at 1:10 AM Ashutosh Bapat
   > <ashutosh.bapat.oss@gmail.com> wrote:
   > > I need to sign in to github to add my review comments. So those who do not have a github account can not use it for review. But I don't think that can be fixed. We need a way to know who left review comments.
   >
   > I don't think Jelte was talking about moving review discussion to
   > Github, just providing a link to *view* the patches there.
   
   Totally correct. And I realize now I should have called that out
   explicitly in the initial email.
   
   While I personally would love to be able to read & write comments on a
   Github PR, integrating that with the mailing list in a way that the
   community is happy with as a whole is no small task (both technically
   and politically).
   
   So (for now) I took the easy way out and sidestepped all those
   difficulties, by making the github branches of the cfbot (which we
   already had) a bit more user friendly as a way to access patches in a
   read-only way.
   
   >  Now I'm
   > wondering if there is a way to disable comments on commits in the
   > postgresql-cfbot GH account.  I guess they'd be lost after 48 hours
   > anyway when the branch gets force-pushed and commit hash changes?  I
   > don't want people to start posting comments there that no one is
   > looking at.
   
   It seems you can disable them for 6 months at a time here:
   https://github.com/postgresql-cfbot/postgresql/settings/interaction_limits
   
   
   
   

8. Re: cfbot update: Using GitHub for patch review

   Ashutosh Bapat <ashutosh.bapat.oss@gmail.com> — 2024-07-01T10:01:38Z

   On Sat, Jun 29, 2024 at 2:12 PM Jelte Fennema-Nio <postgres@jeltef.nl>
   wrote:
   
   > On Sat, 29 Jun 2024 at 01:13, Thomas Munro <thomas.munro@gmail.com> wrote:
   > >
   > > On Sat, Jun 29, 2024 at 1:10 AM Ashutosh Bapat
   > > <ashutosh.bapat.oss@gmail.com> wrote:
   > > > I need to sign in to github to add my review comments. So those who do
   > not have a github account can not use it for review. But I don't think that
   > can be fixed. We need a way to know who left review comments.
   > >
   > > I don't think Jelte was talking about moving review discussion to
   > > Github, just providing a link to *view* the patches there.
   >
   > Totally correct. And I realize now I should have called that out
   > explicitly in the initial email.
   >
   
   While I personally would like to see that one day, getting a consensus and
   changing the process for whole community is a lot of effort. I didn't think
   (or mean) that we would move our review process to Github with this change.
   Sorry if it sounded like that.
   
   
   >
   > While I personally would love to be able to read & write comments on a
   > Github PR, integrating that with the mailing list in a way that the
   > community is happy with as a whole is no small task (both technically
   > and politically).
   >
   
   It is not a small amount of work, I agree. But it may be a way forward.
   Those who want to use PR for review can review them as long as the reviews
   are visible on the mailing list. Many of us already draft our review emails
   similar to how it would look like in a PR. If the PR system can send that
   email on reviewer's behalf (as if it's sent by the reviewer) it will
   integrate well with the current process. People will learn, get used to it
   and move eventually to PR based reviews.
   
   
   >
   > So (for now) I took the easy way out and sidestepped all those
   > difficulties, by making the github branches of the cfbot (which we
   > already had) a bit more user friendly as a way to access patches in a
   > read-only way.
   >
   
   +1.
   
   
   -- 
   Best Wishes,
   Ashutosh Bapat