New process of getting changes into the commitfest app

<copy pasting Jacob Brazeal's full response to the original version of
this email here, to keep the discussion in one thread>

On Thu, 23 Jan 2025 at 01:25, Jacob Brazeal <jacob.brazeal@gmail.com> wrote:

Magnus wants reviews before deployment to be required, in an effort to
get as close-to-perfect commits as possible. I, on the other hand,
think that the benefit of close-to-perfect commits is not worth the
delays in deploying that those reviews currently introduce. I'd rather
deploy code more often to get feedback from users, and make quick
improvements/fixes based on that feedback. (this "deploy early, fix
quickly" approach is also what's being done for cfbot repo and I
haven't heard any complaints about it)

Perhaps we could de-risk this change by adding some automated tests in CI. I'm happy to help with this effort.

Yeah, some automated tests would be great.

"Big" changes should bake in the staging environment for at least a week.

Would we allow the staging and production branches to diverge very much? I think the value for automated testing would increase in this case.

Yeah, that's a great question. My suggestion would be to have the prod
branch simply be a delayed version of the staging branch. So basically
the flow for normal changes would be:
1. Create a PR
2. Get reviews and address those
3. Get approval.
4. Change gets merged to the staging branch (using a squash commit or rebase)
5. Staging branch gets deployed to staging server
6. Wait a week if the PR was "Big"
7. Staging branch gets merged into prod branch using a fast-forward-only merge
8. Prod branch gets deployed to prod server

Any problems found on staging would be solved by additional
commits/PRs, not by amending commits+force-pushes. That way new
contributions can always be based on the staging branch. A benefit of
this is that contributors will then automatically do some additional
local testing of changes that are on staging, but not yet deployed to
prod.

This seems like a great idea !

Maybe we can start out by adding some basic CI tests on the mirror
repository to sort of 'dry run' the new process?
I'll be happy to submit a PR with some basic tests on the repository.

Regards,
Akshat Jaimini

On Thu, Jan 23, 2025 at 6:48 PM Jelte Fennema-Nio <postgres@jeltef.nl>
wrote:

23.01.2025 15:57, Jelte Fennema-Nio пишет:

(Resent because sending to both -hackers and -www gets emails put in
the moderation queue, and I don't want to introduce that delay to all
replies. If you received the previous version because you're in the CC
please only reply to this one)

# Background

As some of you might have noticed I've been trying to breathe some
more life into development on the commitfest app[1], both by
contributing myself but also by encouraging contributions of others.
Basically I'd like to become one of the maintainers of the commitfest
app project. The process to get there has been much more of a struggle
than I'd hoped...

...

I requested Magnus to give me commit access to the pgcommitfest repo
so that I could deploy improvements without having to wait for his
reviews.

Given history of libxz backdoor, I'd fear to give "commit access" for
anything critical to rather fresh member of community.

I'm not in core-team though.

On Mon, 27 Jan 2025 at 03:09, Yura Sokolov <y.sokolov@postgrespro.ru> wrote:

23.01.2025 15:57, Jelte Fennema-Nio пишет:

(Resent because sending to both -hackers and -www gets emails put in
the moderation queue, and I don't want to introduce that delay to all
replies. If you received the previous version because you're in the CC
please only reply to this one)

# Background

As some of you might have noticed I've been trying to breathe some
more life into development on the commitfest app[1], both by
contributing myself but also by encouraging contributions of others.
Basically I'd like to become one of the maintainers of the commitfest
app project. The process to get there has been much more of a struggle
than I'd hoped...

...

I requested Magnus to give me commit access to the pgcommitfest repo
so that I could deploy improvements without having to wait for his
reviews.

Given history of libxz backdoor, I'd fear to give "commit access" for
anything critical to rather fresh member of community.

+1 in github you can enforce a minimum number of reviewers. IMO there
should be a minimum of two reviewers and one of the reviewers should
be from the security group/role. Though primary risk would be
introducing new vulnerable dependency but there is no bound to other
kinds of exploitation. Also github vulnerability scan should be
enabled by default.

I'm not in core-team though.

--
Umar Hayat
Bitnine (https://bitnine.net/)

On Sun, 26 Jan 2025 at 19:09, Yura Sokolov <y.sokolov@postgrespro.ru> wrote:

Given history of libxz backdoor, I'd fear to give "commit access" for
anything critical to rather fresh member of community.

That's definitely a valid concern in the general case, but I wouldn't
call myself a fresh member of the community. I've been the primary
maintainer of the PgBouncer repo for ~2 years now and I also have
commit access to the cfbot repo. So *if* I wanted to add backdoor in
some critical infrastructure I wouldn't need access to the commitfest
app repo to do that. I also rank relatively high on Robbert's yearly
stats list[1]http://rhaas.blogspot.com/2025/01/who-contributed-to-postgresql.html.

[1]: http://rhaas.blogspot.com/2025/01/who-contributed-to-postgresql.html

On Mon, 27 Jan 2025 at 05:38, Umar Hayat <postgresql.wizard@gmail.com> wrote:

+1 in github you can enforce a minimum number of reviewers. IMO there
should be a minimum of two reviewers and one of the reviewers should
be from the security group/role.

In a perfect world I'd agree, but I don't think there are currently
enough people involved in the project to make two reviewers a
realistic option.

Though primary risk would be
introducing new vulnerable dependency but there is no bound to other
kinds of exploitation. Also github vulnerability scan should be
enabled by default.

Enabled that now on my Github mirror. I don't think it'll actually do
anything though. We don't pin exact python dependency versions,
because on prod we only use Python dependencies available in Debian
(which should resolve security issues).

On Sat, 25 Jan 2025 at 06:29, Akshat Jaimini
<akshatjpostgresql@gmail.com> wrote:

I'll be happy to submit a PR with some basic tests on the repository.

Sounds good, what basic tests do you have in mind?

I have this auto-formatting PR open that also adds some github
actions: https://github.com/JelteF/commitfest/pull/1

On Thu, Jan 23, 2025 at 7:57 AM Jelte Fennema-Nio <postgres@jeltef.nl> wrote:

(Resent because sending to both -hackers and -www gets emails put in
the moderation queue, and I don't want to introduce that delay to all
replies. If you received the previous version because you're in the CC
please only reply to this one)

# Background

As some of you might have noticed I've been trying to breathe some
more life into development on the commitfest app[1], both by
contributing myself but also by encouraging contributions of others.
Basically I'd like to become one of the maintainers of the commitfest
app project. The process to get there has been much more of a struggle
than I'd hoped...

So far I've been sending patches privately to Magnus (who is currently
the only maintainer afaik). Sadly, Magnus usually has a lot on his
plate so he often takes a while to respond. This has made the
turn-around on getting my patches deployed pretty long (even for minor
patches).

I requested Magnus to give me commit access to the pgcommitfest repo
so that I could deploy improvements without having to wait for his
reviews. He didn't think that was a good idea. It turns out we
fundamentally disagree on what's acceptable way to deploy:

Magnus wants reviews before deployment to be required, in an effort to
get as close-to-perfect commits as possible. I, on the other hand,
think that the benefit of close-to-perfect commits is not worth the
delays in deploying that those reviews currently introduce. I'd rather
deploy code more often to get feedback from users, and make quick
improvements/fixes based on that feedback. (this "deploy early, fix
quickly" approach is also what's being done for cfbot repo and I
haven't heard any complaints about it)

After some private emails back-and-forth, one thing that we both do
agree on is that it would be good to move both development and
discussion of new features into the open. So I thought it would be
good to also discuss in the open, what exactly that should look like.

# Proposal for new process

I think the following set of guidelines would be a good start:

1. Development happens on GitHub using Pull Requests (PRs) and Issues
(see last section for why GitHub). Currently that's on my mirror of
the repo[2], but that could be moved under the official "postgres"
GitHub org.
2. Chat discussions around development happen on the #commitfest-dev
channel on the PostgreSQL Hacker Mentoring Discord[3].
3. Ideas for new features should be discussed on the pgsql-hackers
mailinglist to make sure that the users of the commitfest app don't
hate the idea.
4. Small incremental improvements to existing features and bugfixes
don't need pgsql-hackers involvement.
5. Reverts and trivial fixes (typos/forgotten check for None/etc) can
be deployed without review
6. "Big" changes should bake in the staging environment for at least a week.
7. If you're deploying a change, you're expected to be reasonably
available in the next few days after to resolve issues (by reverting
or hotfixing).
8. Feedback/complaints about newly deployed changes can be given on
pgsql-hackers (CC-ing the committer & author) or through GitHub
issues.

I am personally in favor of a more open process like this. The
commitfest app seems like the right piece of infrastructure to try
this with. And we definitely want to spread out the maintenance burden
-- not concentrate it. Now, granted, I know nothing about the CF app
code or deployment process. But it sure seems like a win to remove
bottlenecks and enable well-established contributors like you to
support pieces of our infrastructure. Even if the CF app is down one
day a week, I think that would be fine. It's not like our docs being
down. It's a tool for Postgres developers. I think the bigger problem
would be if it went down for weeks on end and you were MIA and no one
knew how to fix your code. But, I somehow don't think that will
happen. I think it is more important to our community that we enable
more people to support development than it is that every tool we have
is perfectly stable.

- Melanie

On Thu, 23 Jan 2025 at 13:57, Jelte Fennema-Nio <postgres@jeltef.nl> wrote:

# Proposal for new process

I just announced an upcoming commitfest app release[1]/messages/by-id/CAGECzQTbHhaZMY-=Y=OGMU_jik4MqCfsc7rDf6AnT=Aq-B6cZA@mail.gmail.com, following the
discussion on this topic at the FOSDEM dev meeting.

The rest of the conclusion is that we'll roughly follow the approach
outlined in this original email. My GitHub mirror is now moved to the
official postgres github org[2]https://github.com/postgres/pgcommitfest. I also opened a bunch of GitHub
issues[3]https://github.com/postgres/pgcommitfest/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen to track features/changes we want based on in-person
discussions. Some of those should be pretty easy to resolve (indicated
by the good-first-issue label).

[1]: /messages/by-id/CAGECzQTbHhaZMY-=Y=OGMU_jik4MqCfsc7rDf6AnT=Aq-B6cZA@mail.gmail.com
[2]: https://github.com/postgres/pgcommitfest
[3]: https://github.com/postgres/pgcommitfest/issues?q=sort%3Aupdated-desc+is%3Aissue+is%3Aopen