On 09.01.2023 at 21:18, Tom Lane wrote:
> It's not great to have multiple CF entries pointing at the same email
> thread --- it confuses both people and bots. Next time please split
> off a thread for each distinct patch.
I agree. I had overestimated the cfbot's ability to handle branched
threads. I'll create separate threads next time.
> * AFAIK our practice is to use "-" never "_" in XML ID attributes.
> You weren't very consistent about that even within this patch, and
> the overall effect would have been to have no standard about that
> at all, which doesn't seem great. I changed them all to "-".
Noted. Maybe it's worth to write a short paragraph about Ids and their
style somewhere in the docs (e.g. Appendix J.5).
> * I got rid of a couple of "-et-al" additions, because it did not
> seem like a good precedent. That would tempt people to modify
> existing ID tags when adding variables to an entry, which'd defeat
> the purpose I think.
I tried to use it sparsely, mostly where a varlistentry had multiple
child items and I had arbitrarily pick one of them. It's not important,
though. I'm curious how you solved this.
> * I fixed a couple of things that looked like typos or unnecessary
> inconsistencies. I have to admit that my eyes glazed over after
> awhile, so there might be remaining infelicities.
I'm all for consistency. The only places where I intentionally refrained
from being consistent was where I felt Ids would get too long or where
there were already ids in place that didn't match my naming scheme.
> It's probably going to be necessary to have follow-on patches,
> because I'm sure there is stuff in the pipeline that adds more
> ID-less tags. Or do we have a way to create warnings about that?
Agreed. And yes, we do have a limited way to create warnings (that's
part of the other patch).
> I'm unqualified to review CSS stuff, so you'll need to get somebody
> else to review that patch. But I'd suggest reposting it, else
> the cfbot is going to start whining that the patch-of-record in
> this thread no longer applies.
I will do that. Thanks for your feedback!
Regards,
Brar