Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Fix capitalization #452

Merged
merged 4 commits into from Feb 28, 2017
Merged

Fix capitalization #452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b52b31b
Remove code that is never executed
Feb 3, 2017
4b1268c
Always output lowercase to requirements file
Feb 9, 2017
e4e0334
Update writer test to lowercase only
Feb 21, 2017
74484a0
Rename writer test to better reflect what happens
Feb 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 0 additions & 8 deletions piptools/scripts/compile.py
View file
Open in desktop
Expand Up @@ -129,8 +129,6 @@ def cli(verbose, dry_run, pre, rebuild, find_links, index_url, extra_index_url,
# over the stuff in the requirements files
upgrade_packages = [InstallRequirement.from_line(pkg)
for pkg in upgrade_packages]
upgrade_pkgs_by_key = {key_from_req(ireq.req): ireq
for ireq in upgrade_packages}

# Proxy with a LocalRequirementsRepository if --upgrade is not specified
# (= default invocation)
Expand All @@ -140,12 +138,6 @@ def cli(verbose, dry_run, pre, rebuild, find_links, index_url, extra_index_url,
for ireq in ireqs:
key = key_from_req(ireq.req)

# Packages explicitly listed on the command line should not remain
# pinned by whatever is in the dst_file (the command line argument
# overwrites the current pins)
if key in upgrade_pkgs_by_key:
ireq = upgrade_pkgs_by_key[key]

if is_pinned_requirement(ireq):
existing_pins[key] = ireq
repository = LocalRequirementsRepository(existing_pins, repository)
Expand Down
4 changes: 2 additions & 2 deletions piptools/utils.py
View file
Open in desktop
Expand Up @@ -73,9 +73,9 @@ def format_requirement(ireq, include_specifier=True):
if ireq.editable:
line = '-e {}'.format(ireq.link)
elif include_specifier:
line = str(ireq.req)
line = str(ireq.req).lower()
else:
line = name_from_req(ireq.req)
line = name_from_req(ireq.req).lower()
return line


Expand Down
4 changes: 2 additions & 2 deletions tests/test_writer.py
View file
Open in desktop
Expand Up @@ -36,14 +36,14 @@ def test_format_requirement_annotation(from_line, writer):
'test==1.2 ' + comment('# via xyz'))


def test_format_requirement_annotation_case_sensitive(from_line, writer):
def test_format_requirement_annotation_lower_case(from_line, writer):
ireq = from_line('Test==1.2')
reverse_dependencies = {'test': ['xyz']}

assert (writer._format_requirement(ireq,
reverse_dependencies,
primary_packages=[]) ==
'Test==1.2 ' + comment('# via xyz'))
'test==1.2 ' + comment('# via xyz'))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is made to explicitly validate that case is NOT changed. (test is named test_format_requirement_annotation_case_sensitive). Are we sure this is ok ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Case sensitivity is not the same as unchanged case. On the other hand, neither pip, nor pypi, nor pip-tools (as it uses pip itself) are case sensitive. Maybe it's better to either just get rid of this test, or rename it, or even write another one?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes that makes sense.

Going forward, I think adding and maintaining tests is really important, particularly in this new "distributed" maintainership model (jazzband) that pip-tools is using. I know it is more work, but it is better to spend time locally than distribute the debugging effort on the internet :-).

Maybe this should be included in the 1.8.1 release I am preparing. A release candidate 1 is out already.



def test_format_requirement_not_for_primary(from_line, writer):
Expand Down