Thread: Re: Tags in the commitfest app: How to use them and what tags to add?
On Mon, Jun 23, 2025 at 1:17 AM Jelte Fennema-Nio <me@jeltef.nl> wrote: > The tags that are currently available are some default ones that I > thought might be useful. If you're missing certain tags or don't like > the default ones, please respond to this thread. If you have the right > permissions, you can even create missing tags yourself in the admin > panel.^1 But if you do, I think it would be useful to post here which > ones you added for discussion purposes. Initial thoughts: - "dblink" seems overly specific compared to the others. - "Backport" seems strange. That's what the Version column is for, no? - "Comments Only" feels somehow... standoffish? defensive? How about "Comments [Requested/Needed]" or something similar? > ^1: I don't know who can do this, as I myself currently don't have > that permission, nor the permission to look at permissions... Hmm, should we grant the CFM group the ability to maintain the tags? I don't currently have that permission either. (Who added the existing ones?) --Jacob
On Mon, Jun 23, 2025 at 11:52 AM Jelte Fennema-Nio <me@jeltef.nl> wrote: > On Mon, 23 Jun 2025 at 18:29, Jacob Champion > <jacob.champion@enterprisedb.com> wrote: > > - "dblink" seems overly specific compared to the others. > > It seemed roughly as specific as postgres_fdw to me. Maybe we should > make sure they are grouped more alphabetically. Oh, I actually missed that postgres_fdw was in the list. Originally the tags dropdown menu showed no scrollbar for me, so all I could see or search were the first six or so... I'm not sure what happened, but that changed sometime today. So... we have a lot more tags than I thought we did. I'll have to give the current list some more thought. > > - "Backport" seems strange. That's what the Version column is for, no? > > I still don't know how I'm supposed to use the version column (e.g. > what is the difference between stable and pg19), and it seems out of > date or not filled in half of the time. So I'd rather have it replaced > with tags with clear intent. Maybe have backport tags for each > Postgres version instead of "Backport - PG16" etc. The assumption > being, if it doesn't have a backport tag, then it should go into > master. Personally, I would much rather that information be coded separately from the tags system. I don't want the CF page filled with a bazillion "Backport - 15" "Backport - 16" "Backport - 17" tags... I'd like the tags to convey immediately useful information that we can't currently get somewhere else, and I'd also like them not to rot over time. > > - "Comments Only" feels somehow... standoffish? defensive? How about > > "Comments [Requested/Needed]" or something similar? > > I meant this as "This patch changes only comments", the hover text > also explains that once selected. Oh, I hadn't realized that was hoverable. I'm not sure that's very discoverable at the moment. --Jacob
On Mon, Jun 23, 2025 at 12:01 PM David G. Johnston <david.g.johnston@gmail.com> wrote: > > Yes, categories, and give each category its own line in the table. I'm headed in the opposite direction. Let me elaborate with some very strong opinions about the existing tags. (No one has to share my strong opinions.) - Help - Bikeshedding - Good First Review - Missing Tests This category of tag is the best. It is completely new information, not captured anywhere else in the UI, that is useful at the top level and helps drive reviews forward by helping the community find interesting things. - Bugfix - Security I'm not excited about these tags, but in the absence of a bug tracker, I'm glad we have them. Now it doesn't matter what "section" your patch is in; if you realize it needs to be treated as a bug fix, or it gains some sort of security caveat, you can tag them as such. - dblink - PL/Perl - postgres_fdw I don't like these at all. You can already search the patches with the search box, so introducing a community norm that adds a bunch of "which code did I touch" tags serves to clutter the UI and give new CFMs an excuse to spend a bunch of time categorizing as opposed to moving patches forward. Put the target of your patch in the entry title somewhere -- and if it touches ten sections, I didn't really want to see ten tags anyways. - Backport I am completely against this tag in particular. We have this information already in its own Version column (though it's kind of sad it's not sortable). If that column isn't working for people now, I really doubt that moving the information to tags is going to help in any way; we can either clarify the Version labels or make the meaning discoverable. --Jacob
On Mon, Jun 30, 2025 at 1:20 PM Jacob Champion <jacob.champion@enterprisedb.com> wrote: > This category of tag is the best. It is completely new information, > not captured anywhere else in the UI, that is useful at the top level > and helps drive reviews forward by helping the community find > interesting things. Oh, and while I'm thinking about it, I'd like to propose two new tags in this vein: - "help, I'm stuck in perma-rebase hell" - "this is my first contribution to the project" I would also like to request that CFMs be given the ability to add and edit (but maybe not delete?) tags. --Jacob
Jacob Champion <jacob.champion@enterprisedb.com> writes:
> I'm headed in the opposite direction. Let me elaborate with some very
> strong opinions about the existing tags. (No one has to share my
> strong opinions.)
I do ... in particular,
> - dblink
> - PL/Perl
> - postgres_fdw
>
> I don't like these at all.
As an example of why these aren't too helpful, a patch that adds
a new kind of expression node is likely to touch a large subset
of any such code-area-based tags.
regards, tom lane
On Mon, 30 Jun 2025 at 22:20, Jacob Champion <jacob.champion@enterprisedb.com> wrote: > > On Mon, Jun 23, 2025 at 12:01 PM David G. Johnston > <david.g.johnston@gmail.com> wrote: > > > > Yes, categories, and give each category its own line in the table. > > I'm headed in the opposite direction. Let me elaborate with some very > strong opinions about the existing tags. I definitely agree with your general ranking of usefulness. > - dblink > - PL/Perl > - postgres_fdw > > I don't like these at all. You can already search the patches with the > search box Honestly I had never used this search box before you mentioned it (initially, I even thought you meant the one on the homepage, and that one IS pretty useless for these kind of searches). I think maybe we should show the search/filter bar by default, because I keep forgetting it exists and I continue to use regular Ctrl+F instead. > "which code did I touch" tags serves to clutter the UI and give new > CFMs an excuse to spend a bunch of time categorizing as opposed to > moving patches forward. Put the target of your patch in the entry > title somewhere -- and if it touches ten sections, I didn't really > want to see ten tags anyways. I'm not so sure I like these either. I think the main reason I added these was because I wanted to see what it's like to replace the "Topic" system with tags. And I tried adding a few more to gauge what level of depth would still be useful. I agree that this level is too much. I quite dislike the current topic system. Partially because it's impossible to filter by a topic (like you can now do with tags), but primarily because the actual available topics very often overlap, and a patch ends up in a random one. For instance patches related to vacuum end up in 6 distinct topics[1] Part of the reason for the overlap seems to be that there are multiple purposes topics are used for: 1. My change is impacting this code area/featureset: SQL commands, Replication & Recovery, System Administration, Monitoring & Control, Clients, Procedural languages 2. I'm improving existing code: Bugfix, Security, Performance 3. This doesn't change behaviour: Docs, Comments, Testing, Refactoring 4. None of the oher topics really fit, but I was required to pick one: Server features, Miscellaneous Obviously 1 often overlaps with 2 or 3. It's even quite common for 2 and 3 to overlap internally. So I think replacing those to be tags makes a lot of sense. Regarding 1 I do feel we miss a bunch of broad categories here. At the very least "Extensibility" would be useful imo, but I think there's other topics that would be nice too, like "Vacuum" or "Planner". Regarding 4 I don't really see a point in having them, and definitely not in having 2 of them. [1]: https://commitfest.postgresql.org/53/?text=vacuum&status=-1&targetversion=-1&author=-1&reviewer=-1&sortkey= > - Backport > > I am completely against this tag in particular. We have this > information already in its own Version column (though it's kind of sad > it's not sortable). If that column isn't working for people now, I > really doubt that moving the information to tags is going to help in > any way; we can either clarify the Version labels or make the meaning > discoverable. I think this is fair. I think being able to sort by the version column would help a lot, but to me the main problem is that it's unclear what the version there is actually supposed to mean. Primarily because afaict people currently use the same version in at least four different ways depending on the dev cycle: 1. During PG18 dev cycle, someone might add the PG19 version to a patch they want to have in the cf app, but don't intend to be shippable. 2. During the PG19 cycle, it means "something I intend to push through this cycle" 3. During the PG20 cycle, it means "something I want to backport to PG19" 4. Once committed, the version might be changed to the lowest version that the patch was committed to. Unsurprisingly, people don't always update this version throughout the years. So a label that was meant one way, might suddenly start to mean something completely else. Then there's "stable", which afaict is a way of saying something needs to be backported? But it's currently unknown yet how far exactly. And finally there's a lot without a version. Does that mean "the current" cycle, or does it mean the author didn't choose one because it was unclear which one to choose? So a PROPOSAL to clarify the version column: 1. Rename "stable" to "backport" 2. Add some info to the help page, and the label 3. Introduce a "current" and a "next" version, those might still reflect an outdated state, but at least they don't start to mean different things when they are not being updated. 4. Only add a new version to the list once it's a version that can be backported. So after the feature freeze (it should be easy to create these automatically now). 5. Start requiring a version, and replace existing nulls with "current". 6. (optional) capitalize the versions so "PG18" instead of "pg18" and "Backport" instead of "backport"