Add new ssh key argument and don't check magic number in old versions#234
Add new ssh key argument and don't check magic number in old versions#234pablogsal wants to merge 4 commits intopython:mainfrom
Conversation
|
(I pushed a test and lint fix) |
|
|
||
| def check_magic_number(db: ReleaseShelf) -> None: | ||
| release_tag = db["release"] | ||
| if release_tag.major == 3 and release_tag.minor <= 13: |
There was a problem hiding this comment.
It would be nice to avoid hardcoding this, as it's another thing that needs adding to a longish list when a new version comes around.
Perhaps we can fetch the newest version from https://github.com/python/devguide/blob/main/include/release-cycle.json (and later from python/peps#4331)?
This can also be a followup PR.
There was a problem hiding this comment.
this is the first version that needed this feature. How fetching the newer version would be cleaner ?
There was a problem hiding this comment.
Is it always only the current feature release that this check will apply for? If so, perhaps expanding the security Boolean to a branch status enum would be better, as then we could test if branch-status == feature here.
A
There was a problem hiding this comment.
No, this particular check applies to all releases that are after we changed that file, that is, anything above or including 3.13
There was a problem hiding this comment.
OK, if the limit is always 3.13 and we don't need to change it, then a hardcoded version makes sense here.
Let's use a tuple so we don't need to worry about Python 4 :)
| if release_tag.major == 3 and release_tag.minor <= 13: | |
| if not release_tag.as_tuple() >= (3, 14): |
There was a problem hiding this comment.
Could this be written using <= ?
There was a problem hiding this comment.
Yes, or rather as < (3, 13). I'll open a new PR.
| parser.add_argument( | ||
| "--security-release", | ||
| dest="security_release", | ||
| action="store_true", | ||
| default=False, | ||
| help="Indicate this is a security release (only checks for Linux files)", | ||
| ) |
There was a problem hiding this comment.
And similarly, we could check the status in the JSON file.
There was a problem hiding this comment.
Here's a PR to check the status from the devguide, so the RM doesn't have to remember to pass --security each time:
Merging that will update this PR.
|
I've split out the |
|
I've split out the remaining changes: |
|
All merged, thanks all! |
4 participants
No description provided.