Thread: Commitfest app release on Feb 17 with many improvements
At the FOSDEM dev meeting we discussed potential improvements to the
commitfest app and how to handle deploying future changes with minimal
disruption to existing workflows. We're going to try a new approach:
announcing a new commitfest release ~2 weeks in advance, including
release notes. That way people can try out the changes on the staging
environment and raise concerns/objections. So hereby such an
announcement:
The next commitfest app release will take place on February 17 and
will contain the following user facing changes:
1. Showing CFBot status on patch and commitfest pages
2. Showing a link to the CFBot github diff on patch and commitfest pages
3. Showing total additions/deletions of the most recent patchset on
patch and commitfest pages
4. Showing additions/deletions of the first patch of the most recent
patchset on the patch page
5. Showing the version and number of patches of the most recent
patchset on the patch page
6. Canonical links for the patch page are now /patch/{patchid} instead
of /{cfid}/{patchid}
7. If you subscribed for emails or patches where you're an author, you
will now get an email when your patch needs a rebase.
8. Clicking headers will toggle sorting, instead of always enabling it
9. Allow sorting patches by name
10. Allow sorting patches by total additions/deletions (added together
for total lines changed)
11. Add ID column to commitfest page (allows sorting)
12. Allow sorting using the header of closed patches too
13. Fix dropdown direction for dropdowns at the bottom of the page
(they now drop up)
14. The previous GiHub and CirrusCI links are now integrated into the
new CFBot status row of the patch page
15. The previous GitHub checkout instructions are now integrated into
the new CFBot status row as a button that copies the commands to the
clipboard
You can try out these changes on the staging environment[1] with
pgtest:pgtest as user & password for the http auth. If you want to
make edits you need to create a new account, read-only doesn't need an
account.
[1]: https://commitfest-test.postgresql.org/
On Fri, Jan 31, 2025 at 03:23:56PM +0100, Jelte Fennema-Nio wrote:
> At the FOSDEM dev meeting we discussed potential improvements to the
> commitfest app and how to handle deploying future changes with minimal
> disruption to existing workflows. We're going to try a new approach:
> announcing a new commitfest release ~2 weeks in advance, including
> release notes. That way people can try out the changes on the staging
> environment and raise concerns/objections. So hereby such an
> announcement:
Thanks for doing this!
> The next commitfest app release will take place on February 17 and
> will contain the following user facing changes:
>
> 1. Showing CFBot status on patch and commitfest pages
> 2. Showing a link to the CFBot github diff on patch and commitfest pages
> 3. Showing total additions/deletions of the most recent patchset on
> patch and commitfest pages
> 4. Showing additions/deletions of the first patch of the most recent
> patchset on the patch page
> 5. Showing the version and number of patches of the most recent
> patchset on the patch page
> 6. Canonical links for the patch page are now /patch/{patchid} instead
> of /{cfid}/{patchid}
> 7. If you subscribed for emails or patches where you're an author, you
> will now get an email when your patch needs a rebase.
> 8. Clicking headers will toggle sorting, instead of always enabling it
> 9. Allow sorting patches by name
> 10. Allow sorting patches by total additions/deletions (added together
> for total lines changed)
> 11. Add ID column to commitfest page (allows sorting)
> 12. Allow sorting using the header of closed patches too
> 13. Fix dropdown direction for dropdowns at the bottom of the page
> (they now drop up)
> 14. The previous GiHub and CirrusCI links are now integrated into the
> new CFBot status row of the patch page
> 15. The previous GitHub checkout instructions are now integrated into
> the new CFBot status row as a button that copies the commands to the
> clipboard
I'm not sure #4 will be very helpful, but the rest looks reasonable to me.
--
nathan
On 2025-Jan-31, Jelte Fennema-Nio wrote: > 3. Showing total additions/deletions of the most recent patchset on > patch and commitfest pages > 4. Showing additions/deletions of the first patch of the most recent > patchset on the patch page > 5. Showing the version and number of patches of the most recent > patchset on the patch page I think this has potential for excessive clutter. I propose that, if we're going to have a column with addition/deletions, we have two things: 1. the total number of addition and deletions in the whole patch series, for the last patchset version posted. This would be shown in the column. 2. a per-patch breakdown of the number of addition and deletions for the last patchset version posted, one line per patch. This would appear in a tooltip on the column for the totals, to avoid interface clutter. > 10. Allow sorting patches by total additions/deletions (added together > for total lines changed) Seems reasonable, though I don't see much of an use-case for this. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Postgres is bloatware by design: it was built to house PhD theses." (Joey Hellerstein, SIGMOD annual conference 2002)
On Tue, 4 Feb 2025 at 17:27, Álvaro Herrera <alvherre@alvh.no-ip.org> wrote: > I think this has potential for excessive clutter. I propose that, if > we're going to have a column with addition/deletions, we have two > things: > > 1. the total number of addition and deletions in the whole patch > series, for the last patchset version posted. This would be shown in > the column. To be clear: the only stats column currently in the staging environment shows exactly this. Changes 4 and 5 are *not* visible in columns on the commitfest page, that info is currently only visible on the patch details page. (For context for others: during the FOSDEM meeting 4 was a column on the commitfest page, but I reverted that after the feedback there) So, I guess clutter wise the current staging is acceptable to you. > 2. a per-patch breakdown of the number of addition and deletions for the > last patchset version posted, one line per patch. This would appear > in a tooltip on the column for the totals, to avoid interface > clutter. Statistics for each patch would definitely be nice. Not trivial to add though, because it would need cfbot to get those statistics for each patch. I created a github issue for this idea to track it[1] > > 10. Allow sorting patches by total additions/deletions (added together > > for total lines changed) > > Seems reasonable, though I don't see much of an use-case for this. My main idea behind this is to allow people to easily find some small patchsets, for when they have ~30 minutes to review something. Or for new contributors to find something to review without getting overwhelmed. [1]: https://github.com/postgres/pgcommitfest/issues/24
On 2025-Feb-04, Jelte Fennema-Nio wrote: > To be clear: the only stats column currently in the staging > environment shows exactly this. Changes 4 and 5 are *not* visible in > columns on the commitfest page, that info is currently only visible on > the patch details page. (For context for others: during the FOSDEM > meeting 4 was a column on the commitfest page, but I reverted that > after the feedback there) > > So, I guess clutter wise the current staging is acceptable to you. Check. > On Tue, 4 Feb 2025 at 17:27, Álvaro Herrera <alvherre@alvh.no-ip.org> wrote: > > 2. a per-patch breakdown of the number of addition and deletions for the > > last patchset version posted, one line per patch. This would appear > > in a tooltip on the column for the totals, to avoid interface > > clutter. > > Statistics for each patch would definitely be nice. Not trivial to add > though, because it would need cfbot to get those statistics for each > patch. I created a github issue for this idea to track it[1] Understood, thanks. > > > 10. Allow sorting patches by total additions/deletions (added together > > > for total lines changed) > > > > Seems reasonable, though I don't see much of an use-case for this. > > My main idea behind this is to allow people to easily find some small > patchsets, for when they have ~30 minutes to review something. Or for > new contributors to find something to review without getting > overwhelmed. Ah, okay, sounds good. -- Álvaro Herrera 48°01'N 7°57'E — https://www.EnterpriseDB.com/
On Fri, 31 Jan 2025 at 15:23, Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > The next commitfest app release will take place on February 17 and > will contain the following user facing changes: In my part of the world it's February 17th now. So this is now deployed. A big recommendation, check the "Notify on all where author" checkbox on your profile page[1]. That way you'll get notified when one of your patches needs a rebase. The cfbot statuses will get filled in over the next few (12?) hours. The first few are already available[2]. If you notice any problems, please let me know. Since the last message on this thread I also fixed the "claim as committer" button, which was broken due to the new patch URLs. > 13. Fix dropdown direction for dropdowns at the bottom of the page > (they now drop up) I didn't mention it in my previous email, but this was fixed by Jacob Brazeal btw. He also added some initial dummy data to the repo to make it much easier for new developers to get started. Thanks Jacob! [1]: https://commitfest.postgresql.org/account/profile/ [2]: https://commitfest.postgresql.org/52/?text=&status=-1&targetversion=-1&author=-1&reviewer=-1&sortkey=6
On Mon, Feb 17, 2025 at 10:03 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > The cfbot statuses will get filled in over the next few (12?) hours. > The first few are already available[2]. If you notice any problems, > please let me know. This looks fantastic. Thanks so much for working on it! And all the other usability improvements too.
On Mon, Feb 17, 2025 at 4:33 PM Thomas Munro <thomas.munro@gmail.com> wrote: > This looks fantastic. Thanks so much for working on it! And all the > other usability improvements too. +1. I very much welcome a more formal release process for the CF app, though with shorter feedback cycles. On that note: It'd be nice if there was a way to see a list of open patches that I am associated with in any way, either as an author or as a reviewer (or as a committer). The list of patches would ideally be organized into two groups: those in the open CF, and those in the next CF. I like "Useful links that you can use and bookmark", but I'd prefer if there was only one link that I needed to bookmark. That is, I'd like it if "Your entries in the current commitfest", "Your entries in the open commitfest", and "Entries that you are reviewing in current commitfest" were all merged into one dashboard view (that also included "Entries that you are reviewing in the open commitfest"). Essentially, "patches that you're involved in in any capacity that haven't already been closed out", organized along the same lines as the existing links (every patch fitting into 1 or those 4 categories), and presented to me as a single page of patches/items. I'm not arguing that you should get rid of "Your entries in the current commitfest", "Your entries in the open commitfest", and "Entries that you are reviewing in current commitfest". Though I will say that if you did get rid of them, I wouldn't object (I'd expect to see the same structure when I viewed the new dashboard view, which would be all I'd ever want to use). Thanks -- Peter Geoghegan
On Tue, Feb 4, 2025 at 05:27:18PM +0100, Álvaro Herrera wrote: > On 2025-Jan-31, Jelte Fennema-Nio wrote: > > > 3. Showing total additions/deletions of the most recent patchset on > > patch and commitfest pages > > 4. Showing additions/deletions of the first patch of the most recent > > patchset on the patch page > > 5. Showing the version and number of patches of the most recent > > patchset on the patch page > > I think this has potential for excessive clutter. I propose that, if > we're going to have a column with addition/deletions, we have two > things: > > 1. the total number of addition and deletions in the whole patch > series, for the last patchset version posted. This would be shown in > the column. > 2. a per-patch breakdown of the number of addition and deletions for the > last patchset version posted, one line per patch. This would appear > in a tooltip on the column for the totals, to avoid interface > clutter. > > > 10. Allow sorting patches by total additions/deletions (added together > > for total lines changed) > > Seems reasonable, though I don't see much of an use-case for this. Looking at the live version, I can sort the "Stats" column from smallest to largest, but not from largest to smallest. Is this intended? -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Do not let urgent matters crowd out time for investment in the future.
On Tue, 18 Feb 2025 at 22:15, Bruce Momjian <bruce@momjian.us> wrote: > Looking at the live version, I can sort the "Stats" column from smallest > to largest, but not from largest to smallest. Is this intended? The next release to prod in ~2 weeks will have that as one of the new features (courtesy of Akshat). You can try it out on the staging website[1] already, with pgtest as user and password for the HTTP auth. [1]: https://commitfest-test.postgresql.org/
On Tue, 18 Feb 2025 at 00:38, Peter Geoghegan <pg@bowt.ie> wrote: > > On Mon, Feb 17, 2025 at 4:33 PM Thomas Munro <thomas.munro@gmail.com> wrote: > > This looks fantastic. Thanks so much for working on it! And all the > > other usability improvements too. > > +1. Thanks all, I'm also very happy that it's deployed. > I very much welcome a more formal release process for the CF app, > though with shorter feedback cycles. Do you mean here that you'd like less than two weeks between the announced staging deploy and prod deploy? > I like "Useful links that you can use and bookmark", but I'd prefer if > there was only one link that I needed to bookmark. That is, I'd like > it if "Your entries in the current commitfest", "Your entries in the > open commitfest", and "Entries that you are reviewing in current > commitfest" were all merged into one dashboard view I think that indeed would be very useful, but also not a small task to do well. The links were pretty easy to implement, so I went for that first. Overall from what I've heard after talking to people there are a few main use cases for the commitfest app. 1. As a committer/reviewer/submitter, keep track of all the patches that you are interested in 2. As a reviewer, find new patches (and from the opposite side, have reviewers find your submitted patches) 3. As a submitter, trigger CFBot runs 4. Get details on a specific patch (does it pass CI, look at a diff in a web view, check out the change locally). 5. If there's other usecases please let me know. I don't use it much for 1 myself. I've been mostly using my "Interesting" label in my email client for tracking interesting threads. So instead I focussed mostly on improving 4, and a bit of 2 for now. Now that there's some more useful patch info available for each patch, I probably also want to start keeping track of patches that I'm interested in, on the cf app.
On Tue, 18 Feb 2025 at 23:57, Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > > I like "Useful links that you can use and bookmark", but I'd prefer if > > there was only one link that I needed to bookmark. That is, I'd like > > it if "Your entries in the current commitfest", "Your entries in the > > open commitfest", and "Entries that you are reviewing in current > > commitfest" were all merged into one dashboard view > > I think that indeed would be very useful, but also not a small task to > do well. The links were pretty easy to implement, so I went for that > first. I created an issue to not forget about that: https://github.com/postgres/pgcommitfest/issues/36
On Tue, Feb 18, 2025 at 11:39:54PM +0100, Jelte Fennema-Nio wrote: > On Tue, 18 Feb 2025 at 22:15, Bruce Momjian <bruce@momjian.us> wrote: > > Looking at the live version, I can sort the "Stats" column from smallest > > to largest, but not from largest to smallest. Is this intended? > > The next release to prod in ~2 weeks will have that as one of the new > features (courtesy of Akshat). > > You can try it out on the staging website[1] already, with pgtest as > user and password for the HTTP auth. > > [1]: https://commitfest-test.postgresql.org/ Yes, works perfectly in test, thanks. -- Bruce Momjian <bruce@momjian.us> https://momjian.us EDB https://enterprisedb.com Do not let urgent matters crowd out time for investment in the future.
On Wed, Feb 19, 2025 at 11:57 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > On Tue, 18 Feb 2025 at 00:38, Peter Geoghegan <pg@bowt.ie> wrote: > > On Mon, Feb 17, 2025 at 4:33 PM Thomas Munro <thomas.munro@gmail.com> wrote: > > > This looks fantastic. Thanks so much for working on it! And all the > > > other usability improvements too. > > > > +1. > > Thanks all, I'm also very happy that it's deployed. Now that we have CI information in an interactive webapp, do you think it would make sense to offer ways to control it a bit? [ ] Enable optional NetBSD task [ ] Enable optional OpenBSD task [ ] Enable optional Windows + MinGW task [ ] Request fresh CI run Until we have the new proper API endpoint up and running, perhaps we could include hidden comments on the page the cfbot scrapes to synchronise its submission list... The last one would affect cfbot's scheduling choices, ie do this one next please, and could be cleared as soon as a new CI result arrives. I suppose the other ones could cause cfbot to add "ci-os-also:" annotations to its commit message, like the existing "ci-os-only:", except not exclusive. You probably know if a patch has portability implications and want to turn those on, but I assume they are still too slow and expensive to enable by default.
> At the FOSDEM dev meeting we discussed potential improvements to the
> commitfest app and how to handle deploying future changes with minimal
> disruption to existing workflows. We're going to try a new approach:
> announcing a new commitfest release ~2 weeks in advance, including
> release notes. That way people can try out the changes on the staging
> environment and raise concerns/objections. So hereby such an
> announcement:
>
> The next commitfest app release will take place on February 17 and
> will contain the following user facing changes:
>
> 1. Showing CFBot status on patch and commitfest pages
> 2. Showing a link to the CFBot github diff on patch and commitfest pages
> 3. Showing total additions/deletions of the most recent patchset on
> patch and commitfest pages
> 4. Showing additions/deletions of the first patch of the most recent
> patchset on the patch page
> 5. Showing the version and number of patches of the most recent
> patchset on the patch page
> 6. Canonical links for the patch page are now /patch/{patchid} instead
> of /{cfid}/{patchid}
> 7. If you subscribed for emails or patches where you're an author, you
> will now get an email when your patch needs a rebase.
> 8. Clicking headers will toggle sorting, instead of always enabling it
> 9. Allow sorting patches by name
> 10. Allow sorting patches by total additions/deletions (added together
> for total lines changed)
> 11. Add ID column to commitfest page (allows sorting)
> 12. Allow sorting using the header of closed patches too
> 13. Fix dropdown direction for dropdowns at the bottom of the page
> (they now drop up)
> 14. The previous GiHub and CirrusCI links are now integrated into the
> new CFBot status row of the patch page
> 15. The previous GitHub checkout instructions are now integrated into
> the new CFBot status row as a button that copies the commands to the
> clipboard
>
> You can try out these changes on the staging environment[1] with
> pgtest:pgtest as user & password for the http auth. If you want to
> make edits you need to create a new account, read-only doesn't need an
> account.
>
> [1]: https://commitfest-test.postgresql.org/
I noticed some of entries are shown with the author field being empty.
e.g. https://commitfest.postgresql.org/patch/5525/
Is this normal?
--
Tatsuo Ishii
SRA OSS K.K.
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp
On Thu, Feb 20, 2025 at 10:53 PM Tatsuo Ishii <ishii@postgresql.org> wrote: > I noticed some of entries are shown with the author field being empty. > e.g. https://commitfest.postgresql.org/patch/5525/ When the layout of https://commitfest.postgresql.org/52/ changed, cfbot's web scraping logic could no longer find the authors :-(. That's a stupid problem to have, that we are going to solve with a new machine-friendly API, but let me try to fix it quickly...
On Thu, 20 Feb 2025 at 11:07, Thomas Munro <thomas.munro@gmail.com> wrote: > > On Thu, Feb 20, 2025 at 10:53 PM Tatsuo Ishii <ishii@postgresql.org> wrote: > > I noticed some of entries are shown with the author field being empty. > > e.g. https://commitfest.postgresql.org/patch/5525/ > > When the layout of https://commitfest.postgresql.org/52/ changed, > cfbot's web scraping logic could no longer find the authors :-(. > That's a stupid problem to have, that we are going to solve with a new > machine-friendly API, but let me try to fix it quickly... I believe Tatsuo was talking about the author field on the commitfest app, not the cfbot. Assuming I'm correct there, I think for the link you shared simply no Author was added when the user created the entry. In the next commitfest release (March 4th), that should not happen anymor (or at least not by accident) because then it will start to autofill yourself as author when you submit a patch.
>> On Thu, Feb 20, 2025 at 10:53 PM Tatsuo Ishii <ishii@postgresql.org> wrote: >> > I noticed some of entries are shown with the author field being empty. >> > e.g. https://commitfest.postgresql.org/patch/5525/ >> >> When the layout of https://commitfest.postgresql.org/52/ changed, >> cfbot's web scraping logic could no longer find the authors :-(. >> That's a stupid problem to have, that we are going to solve with a new >> machine-friendly API, but let me try to fix it quickly... > > I believe Tatsuo was talking about the author field on the commitfest > app, not the cfbot. Yes, I was talking abot the commitfest app. > Assuming I'm correct there, I think for the link you shared simply no > Author was added when the user created the entry. In the next > commitfest release (March 4th), that should not happen anymor (or at > least not by accident) because then it will start to autofill yourself > as author when you submit a patch. Oh, I thought the field was automatically created but looks like it was wrong assumption. Thanks for the explanation. Best reagards, -- Tatsuo Ishii SRA OSS K.K. English: http://www.sraoss.co.jp/index_en/ Japanese:http://www.sraoss.co.jp
On Thu, Feb 20, 2025 at 11:53 PM Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > On Thu, 20 Feb 2025 at 11:07, Thomas Munro <thomas.munro@gmail.com> wrote: > > When the layout of https://commitfest.postgresql.org/52/ changed, > > cfbot's web scraping logic could no longer find the authors :-(. > > That's a stupid problem to have, that we are going to solve with a new > > machine-friendly API, but let me try to fix it quickly... > > I believe Tatsuo was talking about the author field on the commitfest > app, not the cfbot. Oh. Well the author was also missing from the commit messages, and that's now fixed.
On Fri, 31 Jan 2025 at 19:54, Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > > At the FOSDEM dev meeting we discussed potential improvements to the > commitfest app and how to handle deploying future changes with minimal > disruption to existing workflows. We're going to try a new approach: > announcing a new commitfest release ~2 weeks in advance, including > release notes. That way people can try out the changes on the staging > environment and raise concerns/objections. So hereby such an > announcement: > > The next commitfest app release will take place on February 17 and > will contain the following user facing changes: > > 1. Showing CFBot status on patch and commitfest pages I noticed that the "Truncate logs by max_log_size" commitfest entry says it requires a rebase in commitfest at [1]. CFBot says the following error at [2]: ++<<<<<<< ours + <varlistentry id="guc-md5-password-warnings" xreflabel="md5_password_warnings"> + <term><varname>md5_password_warnings</varname> (<type>boolean</type>) + <indexterm> + <primary><varname>md5_password_warnings</varname> configuration parameter</primary> ++======= + <varlistentry id="guc-max-log-size" xreflabel="max_log_size"> + <term><varname>max_log_size</varname> (<type>integer</type>) + <indexterm> + <primary><varname>max_log_size</varname> configuration parameter</primary> ++>>>>>>> theirs But it applies neatly for me and Jim also at [3]. Any idea why patch apply fails with CFbot whereas it passes in our environment? [1] - https://commitfest.postgresql.org/patch/5272/ [2] - http://cfbot.cputube.org/patch_5272.log [3] - https://www.postgresql.org/message-id/4c3a4c2f-fa6b-4d56-8947-fba2c06066fd%40uni-muenster.de Regards, Vignesh
On Thu, 6 Mar 2025 at 16:36, vignesh C <vignesh21@gmail.com> wrote: > But it applies neatly for me and Jim also at [3]. Any idea why patch > apply fails with CFbot whereas it passes in our environment? Okay, the cause of this seems to be that the CFbot currently uses "git apply --3way", not "git am --3way". For some reason "git apply" fails to apply the patch while "git am" succeeds. I'll check this weekend if I can change the logic to use "git am" instead.
Jelte Fennema-Nio <postgres@jeltef.nl> writes:
> Okay, the cause of this seems to be that the CFbot currently uses "git
> apply --3way", not "git am --3way". For some reason "git apply" fails
> to apply the patch while "git am" succeeds. I'll check this weekend if
> I can change the logic to use "git am" instead.
Please see if you can make it use patch(1). IME git is too
stiff-necked about slightly stale patches no matter which
subcommand you use.
regards, tom lane
On Thu, 6 Mar 2025 at 18:10, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Please see if you can make it use patch(1). IME git is too > stiff-necked about slightly stale patches no matter which > subcommand you use. It was using patch(1) in the past for that reason, but with --3way I was able to get "git apply" to apply patches that patch(1) was unable to apply. Also with the git command you get much more useful conflict output.
On Thu, 6 Mar 2025 at 18:39, Jelte Fennema-Nio <postgres@jeltef.nl> wrote: > > On Thu, 6 Mar 2025 at 18:10, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Please see if you can make it use patch(1). IME git is too > > stiff-necked about slightly stale patches no matter which > > subcommand you use. > > It was using patch(1) in the past for that reason, but with --3way I > was able to get "git apply" to apply patches that patch(1) was unable > to apply. Also with the git command you get much more useful conflict > output. Okay, I went for the approach of just trying everything until one works. Starting with "git am", then patch(1), and as a final attempt "git apply". Patch 5272 applies correctly now. I've removed any backoff caused by repeated failures for all existing patches, so all should be retried in the next ~48 hours.