Introduce Drafts page and automatic CF creation #70
Conversation
| A "patch" is a bit of an overloaded term in the PostgreSQL community. Email threads on the mailing list often contain "patch files" as attachments, such a file is often referred to as a "patch". A single email can even contain multiple related "patch files", which are called a "patchset". However, in the context of a CommitFest app a "patch" usually means a "patch entry" in the CommitFest app. Such a "patch entry" is a reference to a mailinglist thread on which change to PostgreSQL has been proposed, by someone sending an email that contain one or more "patch files". The CommitFest app will automatically detect new versions of the patch files and update the "patch entry" accordingly. | ||
| </p> | ||
| <p> | ||
| There are three active categories of patch status: |
There was a problem hiding this comment.
TODO: Somewhere should be "Returned with Feedback"
There was a problem hiding this comment.
In my mind it should either be rejected (or, ideally, withdrawn) or moved back into drafts. Trying to reduce the amount of choice presented to the user by omitting RfW - which isn’t really a descriptive “end state”. If no one accepts that logic it can be easily added.
JelteF
force-pushed
the
introduce-drafts-jelte
branch
from
b9dddce to
f05f885
Compare
This introduce a new type of CommitFest a "Draft" CommitFest. This CommitFest is never "In Progress", but it can be open. It exists for a year. It opens when the final regular CommitFest of the year becomes "In Progress" and stays open for exactly a year. It never becomes "In Progress" itself. Adding a second type of CommitFest also needed a redesign of quite a few things, like the homepage. Also management of the CommitFests needed to be made a bit easier, so admins don't forget to close/create Draft CommitFests. So now, closing/opening/creating CommitFests is done automatically when the time is right for that. A help page is also introduced to explain the CommitFest app. The naming of CommitFests has been changed too. Since we now have a Draft CF every year that needs a name, it seemed reasonable to align the names of the other CFs with that too. So each PG release cycle now has 5 regular commitfests that are called: - PG18-1 - PG18-2 - PG18-3 - PG18-4 - PG18-Final And a single Draft CommitFest, called: - PG18-Draft Finally, it also adds a small initial API endpoint for the CFBot, to request the commitfests that need CI runs. Future PRs will extend this API surface to also include/allow requesting the actual patches that CI should run on. Co-Authored-By: David G. Johnston <david.g.johnston@gmail.com>
JelteF
force-pushed
the
introduce-drafts-jelte
branch
from
f05f885 to
90792d1
Compare
|
Did you have specific rationale for choosing to add a draft Boolean instead of using a status? |
There was a problem hiding this comment.
We push patches written during an in progress cf all the time. Those should be able to be added to the in progress cf by a committer so their cf of record is correct. I.e., move to should show in progress conditionally.
There was a problem hiding this comment.
Interesting... Currently that's not done, but indeed that seems reasonable. I think the easiest way would be to automatically move a patch if it's "Committed" in an "Open" commitfest, to the "In Progress" commitfest if that exists.
|
Not a fan of the fundamental relevant cf getter method having non-cache related side effects that only ever trigger every other month or so. It isn’t even a pure date-related action; our final cf in particular has an end date that is changed during the months it happens in. We should,have a form to make the action easy to perform, but not remove the human component altogether. |
Primarily because being a draft commitfest is a property of the commitfest, not a state it can be in. Once a draft commitfest is closed, it's still a draft commit fest. If we make it a state the only way of knowing that is based on the name. By using a boolean we can also have the full list of commitfests page differentiate between draft commitfests and regular commitfests some way.
I agree it's a bit uncommon to do like this, but I don't see the benefit of requiring a person to press a button every 2 months.
The feature freeze date is decided by the RMT well before the end of March. It seems fine to have them change the enddate on the commitfest app when they email the feature freeze date. |
|
Yeah, I’m ok with draft as a Boolean. Can we just setup a cron job to run daily to do the switchover then? I’d probably still keep the button to press for testing or a fall-back. |
That's pretty much what the CFBot does now, except it's an "every minute" cron job. I guess I could make the refreshing only occur for the API, but I don't really understand what the actual problem is you have with this approach. |
|
Mainly the fundamental POLA violation of having a read-only query modifying state. And why spend overhead on every request checking whether said update is needed or not when 99+% of the time it will not be. Also, it seems problematic to have it be the only way this process gets executed; the fact you have to add a disable flag is just one such consequence. The design is just really an unappealing and unclean combining of unrelated features. |
| </ul> | ||
| And there are three preferred inactive categories of patch status for when | ||
| a patch has been resolved, either by being committed or not: | ||
| And there four "Closed" categories of patch status for when a patch has been |
There was a problem hiding this comment.
And there [are] four “Closed”…
So I kind of agree with that. And if it'd have been easy for me to create a cron job on the machine, I'd probably have done that. Sadly that's not the situation we're in. Getting anything to happen on the infrastructure side is a huge hassle. So I try to avoid that as much as possible. I added code comment with that reasoning. And I'll leave it like this.
The overhead of the check is extremely minimal. So I'm not worried about that at all. It's only 5 comparisons. |
I'll pay the tech debt down if I get annoyed enough. I do appreciate the decision in the face of our organizational dynamics. |
This introduce a new type of CommitFest a "Draft" CommitFest. This
CommitFest is never "In Progress", but it can be open. It exists for a
year. It opens when the final regular CommitFest of the year becomes "In
Progress" and stays open for exactly a year. It never becomes "In
Progress" itself.
Adding a second type of CommitFest also needed a redesign of quite a few
things, like the homepage. Also management of the CommitFests needed to
be made a bit easier, so admins don't forget to close/create Draft
CommitFests. So now, closing/opening/creating CommitFests is done
automatically when the time is right for that. A help page is also
introduced to explain the CommitFest app.
The naming of CommitFests has been changed too. Since we now have a
Draft CF every year that needs a name, it seemed reasonable to align the
names of the other CFs with that too. So each PG release cycle now has 5
regular commitfests that are called:
And a single Draft CommitFest, called:
Finally, it also adds a small initial API endpoint for the CFBot, to
request the commitfests that need CI runs. Future PRs will extend this
API surface to also include/allow requesting the actual patches that CI
should run on.
Fixes #25
Fixes #65