On a temporary branch for this I pointed out that the label features should be feature, to match the term feature release (like the status bugfix matches bugfix release). You could say that it’s not in the scope of this particular PR but I would say that it’s the opportunity to do this minor cleanup 🙂

On a temporary branch for this I pointed out that the label features should be feature, to match the term feature release (like the status bugfix matches bugfix release). You could say that it’s not in the scope of this particular PR but I would say that it’s the opportunity to do this minor cleanup 🙂

Updated!

Will check the other review comments a bit later, thanks all!

Per our previous discussion, I've worked on a suggested revision: CAM-Gerlach@c2cbf6b factoring the dict preprocessing and the actual CSV writing into separate, modular and easily-composable steps, which IMO:

• is a cleaner design (and could be separated into smaller, more atomic functions)
• is a fair bit shorter/simpler
• avoids the code smell of opening two files, creating two CSV writers, having two write calls, etc.
• is more efficient (writing each CSV in one go and only having one file open for a shorter time)
• eliminates the redundant and potentially bug-magnet duplicate CSV_HEADER and listing of keys in the final dict

I also fixed the version sorting issue there.

Thanks, I've integrated that stuff 👍

In addition, I have a few comments/suggestions below, and a couple overall:

• One thing that's missing here is a clear indication of the expected/actual cut-over point between bugfix and security-only releases. Could we add an additional column with this information, and indicate it as such on the chart?

Is it okay to deal with that separately after this PR, to reduce the scope? At least we don't have that information in the current tables.

This is standard for most pages like this, and the Wikipedia page has it. Otherwise, it's not clearly indicated anywhere here when the cutover is planned/has occurred.

The table does, but the chart doesn't. (Aside: it's a bit strange the Wikipedia table has "End of full support" before "End of security fixes".)

• Could we maybe try to keep the JSON key names and order more in sync with the table? I don't really see a compelling reason to not just use the same keys and order, minus formatting (spaces, caps)

Can do!

I think we should keep "pep": 693 instead of "schedule": 693?

So like this?

diff --git a/include/release-cycle.json b/include/release-cycle.json
--- a/include/release-cycle.json
+++ b/include/release-cycle.json
 {
   "3.12": {
     "branch": "main",
     "pep": 693,
     "status": "feature",
-    "release_manager": "Thomas Wouters",
-    "release_date": "2023-10-02",
-    "eol": "2028-10"
+    "first_release": "2023-10-02",
+    "end_of_life": "2028-10",
+    "release_manager": "Thomas Wouters"
   },

• Wouldn't it be more idiomatic to name the Python file with _s? Then it could be imported if needed. Otherwise, doesn't really make sense to have a main() function and an if __name__ == "__main__" block since it isn't possible to import it without importlib hackery.

Yep, renamed! Shall we keep the script in the root or move it to some subdirectory?

• Maybe its just me, but I'd certainly prefer if this was a Sphinx extension integrated into the build as opposed to a standalone script invoked manually or via a non-cross-platform Makefile...

I slightly prefer Makefile, these files won't need updating much, and it would be nice to avoid having to run this for every single make pages iteration when working on something else. Also no need to write/maintain Sphinx extension. If we use a non-cross-platform Makefile, we'll also update the non-cross-platform make.bat for Windows users ;)

But then again there's not much overhead in reading one file and writing two.

What are others' thoughts?

A script with a makefile target (and if possible windows script equivalent, but not a blocker) seems simplest and best to me.

Is it okay to deal with that separately after this PR, to reduce the scope? At least we don't have that information in the current tables.

Unacceptable I say, unacceptable! 😆 Yeah, that seems quite reasonable to me, though its not really up to me either way, heh.

The table does, but the chart doesn't. (Aside: it's a bit strange the Wikipedia table has "End of full support" before "End of security fixes".)

How so, sorry? Since it occurs between initial release and final security EoL, it would seem to me to make the most sense to put it between the two, unless there's something I'm missing...

I think we should keep "pep": 693 instead of "schedule": 693?

So like this?

Sure, that seems to make the most sense to me; in general, I don't have any strong preference what they are, just that they match if practicable. In particular, I realize I was unclear before—I didn't mean to imply that the JSON should be changed to match the names and order of the table, but rather just that one should match the other either way.

Shall we keep the script in the root or move it to some subdirectory?

It seems cleaner to me to move it to an appropriate subdirectory (e.g. _tools, _scripts, etc), though a little less visible/convenient, but I have no strong opinion myself.

I slightly prefer Makefile, these files won't need updating much, and it would be nice to avoid having to run this for every single make pages iteration when working on something else.

Seems fair, so long as the script itself works on Windows and I can just execute it directly if needed. In that case, we of course need to run it in CI to verify the generated output matches is in sync with what is checked in, and that it works without errors (at least for any PRs/pushes touching the script, source or destination files, though since its likely very cheap to run at least on CI, perhaps simpler to just do it unconditionally in an existing workflow).

Updated:

• Rename/reorder data keys
• Move under _tools and add to make.bat
• Test on CI with Windows and Linux
• PEP 257 docstrings

Re: CI

I made a new workflow as the existing workflow doesn't pass on Windows.

Test steps are:

1. Run the script to update the files.
2. Run git status --porcelain to show a list of changed files -> should be empty list.
3. To make the CI fail for changes, run test $(git status --porcelain | wc -l) = 0

This works for Linux but not Windows, I think because of line endings:

 M include/branches.csv
 M include/end-of-life.csv
 M include/release-cycle.mmd

I also added git config core.eol lf and git config core.autocrlf true to try and appease it, but no luck.

@CAM-Gerlach Any Windows tips here?

git config core.autocrlf true will check the files out with CRLF, and the script (assuming it is working correctly) will check them in with LF. If that is indeed the issue here, then git config core.autocrlf input should fix it, which will normalize the line endings on commit but not on checkout. Using that locally on my Windows machine and following the steps in the CI workflow, everything works as expected and the test passes.