[devtools-rfc] Backward compatibility cleanup
Categories
(DevTools :: General, task)
Tracking
(Not tracked)
People
(Reporter: nchevobbe, Unassigned)
References
(Blocks 1 open bug)
Details
At the moment, we're supporting connecting to older server (N-2), which means that often, when changing code on the server, we need to have code handling the backward compatibility on the client.
These backward compatibility lines can be then be removed in N+2 (where N is the revision the code landed in).
We don't have a structured way of dealing with these cleanup. Usually, there will be some comment on the lines, saying they can be removed at some point; and if the author is in a good day, they might file a bug for doing the cleanup.
Having a bug is great, but it's easy to miss as we don't have a reminder when N+2 comes to indicate we can do the work.
As discussed in an early meeting, it would be nice if there was an easy way to retrieve all the different blocks that need to be cleaned for a given revision.
One idea would be to normalize the backward compatibility comment so a quick search would allow to get all the things that can be cleaned.
I opened this rfc mostly to communicate about this and as a way to discuss the templace we should have.
This could probably be something like:
// [devtools-backward-compat | added in 82 | remove in 84] A more detailed comment ...
// [devtools-backward-compat | + Fx82 | - Fx84] A more detailed comment ...
this is a bit long, so let me know if you have better ideas
|
||
Comment 1•4 years ago
|
||
Thanks for opening this rfc!
No strong opinion on the comment format. I think I prefer the first version, less cryptic.
Maybe we should stick to @ rather than [] for code annotations?
Eg // @devtools-backward-compat { added in 82 | remove in 84 } Detailed comment.... Similar size, but looks more consistent with the rest of our js-doc.
I could even imagine a longer variant :)
/**
* @devtools-backward-compat Detailed comment
* @added 82
* @remove 85
*/
We should update https://searchfox.org/mozilla-central/source/devtools/docs/backend/backward-compatibility.md once we decide on something.
|
||
Comment 2•4 years ago
|
||
I'm not sure we should write the "when to remove" (ex: "remove in 84") in code.
This rule can change over time. We rarely change it, but it can and has changed once. We mentioned for example doing a one time compatibility break for fission. We did not, but if we did, we could have removed all existing backward compat code. Thus, ignoring all "remove in xx" comments.
For me, it would make sense to keep the current backward compatibility range as a convention, written only once in docs.
Then, in code, only write the release which introduced the new codepath.
This would help keep the comment short and doesn't refrain copy pasting it everywhere there is something to be removed.
Very often we have to put comments in many places. In specs, in many callsites in fronts, ...
I'm bit worried about the implication of having long comments everywhere.
+1 on using@
What about using // @backward-compat { added in 82 } Detailed comment...?
|
|
||
Comment 3•4 years ago
|
||
Hi Nicolas!
For info, we mentioned this RFC during the Tools Checkin meeting a few weeks ago, and nobody had anything negative to say about the proposal from Alex // @backward-compat { added in 82 } Detailed comment....
So we could go with it and file a bug to update the doc?
Not sure if we also want a meta to update existing backward compat comments.. That would be quite a challenge :D
|
Reporter | |
Comment 4•4 years ago
|
||
(In reply to Julian Descottes [:jdescottes] from comment #3)
Hi Nicolas!
For info, we mentioned this RFC during the Tools Checkin meeting a few weeks ago, and nobody had anything negative to say about the proposal from Alex
// @backward-compat { added in 82 } Detailed comment....So we could go with it and file a bug to update the doc?
Sure, I filed Bug 1677732 for that.
Not sure if we also want a meta to update existing backward compat comments.. That would be quite a challenge :D
That's something that would be nice to do. I guess we could have a mixed-bag bug where we modify existing comments to match the new syntax.
I'll try to do it as well.
I'm closing this bug since we reached a conclusion here, and I'll talk about it in mozilla-central so everyone knows about it.
Thanks!
Description
•