don't merge python_version and python_full_version#382
don't merge python_version and python_full_version#382neersighted merged 1 commit intopython-poetry:mainfrom
Conversation
These markers are not quite equivalent
|
Kudos, SonarCloud Quality Gate passed!
|
neersighted
left a comment
neersighted
left a comment
There was a problem hiding this comment.
LGTM, but I'm curious how we got here -- did everyone just conflate the two of these together and not realize the subtle difference (that python_version is basically always .0, if I understand correctly)? Can we add some documentation somewhere that explicitly defines the intention for each of these (or link to the upstream docs)?
that's not wrong exactly, but thinking that way is pretty much the misconception that I had when introducing the code that this MR is now removing.
|
See, that's definitely needed somewhere in this codebase... Any thoughts on a good place to document it or link to docs? |
|
I don't know where would be a good place to put this. Use of these markers is scattered around the codebases: wherever one wrote this down, I'd think there's a good chance that the next person wouldn't happen to find it. Open to suggestions! Might be better to continue this part of the discussion in python-poetry/poetry#5717 which I intended to cover the general issue, this MR is only a specific example of getting it wrong. |
2 participants
These markers are not quite equivalent; I've updated the relevant testcases to give examples where the previous code got it wrong.
cf python-poetry/poetry#5717
I do not claim that this is the only code that confuses
python_versionandpython_full_version, I just remember this particular example because I wrote it myself!