-
Notifications
You must be signed in to change notification settings - Fork 30
Correct writing of hyphenated and underscored package names #117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Correct writing of hyphenated and underscored package names #117
Conversation
This patch changes hashin such that, If the package specified has underscores in its name, but the new package has hyphens, the following occurs * If there is a new version, hashin updates the package in requirements to be the hyphenated name * If there is no new version, the underscored name remains in place Prior to this patch, the 1st case about would not occur. Instead, hashin would append the new version in its hyphenated form to the requirements file and leave the old version in its underscored form in the requirements file.
|
@peterbe ready for review |
|
Fixes #116 |
peterbe
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did some pretty basic (manual) testing it all seems to work out:
▶ cat /tmp/reqs.txt
readme_renderer==25.0 \
--hash=sha256:cbe9db71defedd2428a1589cdc545f9bd98e59297449f69d721ef8f1cfced68d \
--hash=sha256:cc4957a803106e820d05d14f71033092537a22daa4f406dfbdd61177e0936376
python_memcached==1.58 \
--hash=sha256:4dac64916871bd3550263323fc2ce18e1e439080a2d5670c594cf3118d99b594 \
--hash=sha256:a2e28637be13ee0bf1a8b6843e7490f9456fd3f2a4cb60471733c7b5d5557e4f
▶ python hashin.py -u -i /tmp/reqs.txt
PACKAGE YOUR VERSION NEW VERSION
readme-renderer 25.0 26.0 ✓
python-memcached 1.58 1.59 ✓
▶ cat /tmp/reqs.txt
readme-renderer==26.0 \
--hash=sha256:cbe9db71defedd2428a1589cdc545f9bd98e59297449f69d721ef8f1cfced68d \
--hash=sha256:cc4957a803106e820d05d14f71033092537a22daa4f406dfbdd61177e0936376
python-memcached==1.59 \
--hash=sha256:4dac64916871bd3550263323fc2ce18e1e439080a2d5670c594cf3118d99b594 \
--hash=sha256:a2e28637be13ee0bf1a8b6843e7490f9456fd3f2a4cb60471733c7b5d5557e4f
▶ python hashin.py -u -i /tmp/reqs.txt
▶ echo $?
0
And this one:
▶ python hashin.py -r /tmp/reqs.txt python_memcached==1.58
▶ cat /tmp/reqs.txt
readme-renderer==26.0 \
--hash=sha256:cbe9db71defedd2428a1589cdc545f9bd98e59297449f69d721ef8f1cfced68d \
--hash=sha256:cc4957a803106e820d05d14f71033092537a22daa4f406dfbdd61177e0936376
python-memcached==1.58 \
--hash=sha256:2775829cb54b9e4c5b3bbd8028680f0c0ab695db154b9c46f0f074ff97540eb6
Looks great.
I can't think of any other ways to test it.
I like that if you already had some_package==X.Y.Z where X.Y.Z is the latest version, running hashin -u -i just plainly ignores it. I.e. it doesn't try to correct the name if the version doesn't need to change.
The only thing is the nit.
hashin.py
Outdated
| # in the lines being compared. This results in "old" names matching | ||
| # "new" names so that hashin correctly replaces them when it looks for | ||
| # them. | ||
| rex = re.compile("[-_]") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be...
| rex = re.compile("[-_]") | |
| rex = re.compile(r"[-_]") |
A bit surprised that flake8 didn't complain about that.
Also, rex isn't a great name. It sounds like a pun on "regular expression" and it's not mentioning anything about its purpose in life or its use.
| 3.4: py34 | ||
| 3.5: py35 | ||
| 3.6: py36, lint, restlint | ||
| 3.7: py37, lint, restlint |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note-to-self; we should probably drop 3.4 and add 3.8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you want me to change this in this PR? lemme know
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have a new PR coming that'll also do this. So leave it as is.
hashin.py
Outdated
| # different. | ||
| # The 'new_lines` is what we might intend to replace it with. | ||
| old = set([l.strip(" \\") for l in old_lines]) | ||
| old = set([rex.sub("-", l.strip(" \\")) for l in old_lines]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will fail newer versions of flake8 because l is a bad variable name. It's not your fault but do you mind
changing this to:
| old = set([rex.sub("-", l.strip(" \\")) for l in old_lines]) | |
| old = set([rex.sub("-", line.strip(" \\")) for line in old_lines]) |
whilst you're at it.
|
@peterbe I will send updates shortly. thanks for reviewing! |
* Drop python 3.4 from tox.ini * Add Python 3.8 to tox.ini * Correct variable names * Change string to regex string
fad0d58 to
e7a61dd
Compare
|
changes made. Ready for review |
|
Travis isn't reporting checks here on the PR, but: https://travis-ci.org/github/peterbe/hashin/builds/698623654 is all green! |
This patch changes hashin such that,
If the package specified has underscores in its name, but the new package
has hyphens, the following occurs
be the hyphenated name
Prior to this patch, the 1st case about would not occur. Instead, hashin
would append the new version in its hyphenated form to the requirements file
and leave the old version in its underscored form in the requirements file.