There are several occurrences of the sequence XXX in comments in the code base, both C code and Python code. Every time I encounter an XXX-sequence, I spend time digging through git log trying to find out what message that sequence is supposed to convey. Is it a warning? Is it a TODO comment? Is it a FIXME comment? In my opinion, XXX comments are on par with magical numbers. I’d rather we use plain english to communicate messages to the reader of our source code.
I suggest the following practises to be used instead of XXX sequences:
XXX meaning TODO: create a follow-up issue instead; if the comment is still needed, formulate it using plain english
XXX meaning FIXME: create a follow-up issue instead; if the comment is still needed, formulate it using plain english
XXX meaning WARNING: use the “WARNING”, “Pay attention”, “NB”[1], or similar wording instead
Do we need to put this in PEP-7 and PEP-8, or is it ok to just agree on expressing ourselves clearly?
Linters recognize some patterns such as TODO and XXX. I’m not sure about others. Having some guidelines on writing comments in PEP 7 would be nice. In general, for me, XXX stands for “hey, pay attention to that”. The exact meaning is usually in the comment content.
I concur with Victor. There is nothing cryptic in it to me. I read it as an attention sign. The text of the comment and the context add more details.
There are about 600 occurrences of the XXX comment in the Python code and about 300 occurrences in the C code. I do not think that it was cryptic for people who wrote them.
Of course it is not cryptic for those who wrote them. I’m talking about the interpretation. As a reader, it is not immediately obvious as different core devs put different connotations into these character sequences. It is clearly not aligned with PEP-8, which mandates a clear and unambiguous language.
@erlendaasland You may want to have a look at PEP 350. It explains lots of these code tags and encourages adopting them, but was rejected at the time.
From what I remember, Guido took this tag from X11 code, but I may be wrong. In general, it stands for “There is something to highlight about this section of the code” and the text after it should explain what exactly the issue is.
The text may also reference an open ticket, but this is not necessarily needed. It’s often better to add context right where you put the tag and only guide people to a ticket in case there’s a reason to later resolve the issue. Just redirecting to a ticket without giving context causes too much indirection, IMO, so should be avoided.
Looking at that PEP leaves me even more confused regarding XXX; according to that PEP, I should interpret XXX as a variant of FIXME. IMO, this enhances my case. Nobody really know what it means.
+1
Well, perhaps that was a bad replacement. If you reread my post, you’ll see that I recommend using plain english in most cases.
I’d be quite okay with recommending we use “TODO” and “HACK” instead of “XXX”.
“Warning” ought to be implied in any comment (otherwise why write it?), but the extra mark is useful to indicate “someone should fix this later but I’m not doing it right now” (TODO) or “I know this is bad and here’s the reason why I’m doing it anyway, I won’t mind if someone makes it nicer” (HACK).
Creating an issue for the TODO is usually going to be a good idea, but sometimes it really isn’t helpful (e.g. I’m sure there’s at least one // TODO: Add Windows support if Windows ever supports this in the codebase… probably using XXX and paraphrased)
I don’t quite agree with the meaning giving to it in the PEP either. To me, the XXX code tag simply means: attention, nothing more, nothing less. Could be anything, e.g. a todo item, a known problem with the code in question, something to revisit later, an explanation of why the code is written the way it is, why something was intentionally left out, observations, which are not fully understood yet, but may require more investigation, etc.
Using more precise code tags may make sense, but then again: programmers are lazy [1], so the fallback XXX tag is often used.
Overall, I’d avoid putting too much effort into making this more strict or even setting up some kind of ontology around it. It’s probably good to give a few recommendations, though.
Speaking for myself, I use XXX to signal a wart that is worth paying attention to when maintaining the surrounding code, but is neither a FIXME or TODO.
That is precisely why I use XXX: so that it doesn’t stand out as much as a TODO or FIXME.
(I also tend to think that TODO or FIXME doesn’t have its place in modern codebases: you should use an issue tracker, not bury comments inside the source code)
This discussion clearly shows there are different interpretations of XXX, which proves my point: it is hard to interpret its exact meaning. In most cases that’s ok, but in some cases it is important to know if a comment is a warning about the current code or a hint that something should be fixed (todo, fixme). I still stand by my suggestion to avoid XXX and instead use a clear and unambiguous language.
Usually the language after the XXX is clear and unambiguous. Also there are many cases where it’s not obvious if the situation can be fixed or improved. We don’t want to litter the code with TODOs or FIXMEs that will never be addressed.
Of course not. I don’t think there’s much value in littering the code base with XXX either. Also, and unfortunately, many of the XXX comments are not followed by clear and unambiguous language. I don’t understand the urge to fight writing clear and understandable comments.
Ok, so to make things clear, your complaint is not against the use of “XXX” in itself, but the fact that sometimes it is not followed by a clear explanation? That sounds reasonable to me.
Just because Erlend is the first to bring it up (to this forum), it does not mean he’s the first to find it unclear. It is possible others also find it unclear but did not know how to bring it up or did not find it comfortable to bring it up. Personally I too was confused with the XXX marking. It seems to be a known “code” to people in certain group, but it could be seen as offensive to other people.
XXX can be associated with pornographic or explicit content, which I think is inappropriate for Python open source project. Using “TODO:” or “NOTE:” or “FIXME” would be clearer and more appropriate.
