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.

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

Use lower-case package name in Version #158

Merged
merged 2 commits into from Apr 22, 2017

Conversation

dmurvihill
Copy link

@dmurvihill dmurvihill commented Dec 23, 2016

This commit changes the name 'Treq' in _version.py to 'treq'. This makes the Version capitalization consistent with the caps throughout the package, and makes it possible to use __version__.local() to generate a local version number with incremental.

@codecov-io
Copy link

codecov-io commented Dec 23, 2016

Codecov Report

Merging #158 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #158   +/-   ##
=======================================
  Coverage   96.64%   96.64%           
=======================================
  Files          20       20           
  Lines        1788     1788           
  Branches      153      153           
=======================================
  Hits         1728     1728           
  Misses         39       39           
  Partials       21       21
Impacted Files Coverage Δ
src/treq/_version.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d5cfa3e...34b29f2. Read the comment docs.

@glyph
Copy link
Member

glyph commented Feb 15, 2017

Thanks for your contribution!

My initial reaction is that we should keep it the way it is, because generally when using a package name in this way:

  • It's a proper noun, so capitalization is appropriate
  • Compatibility is a concern, because others might be testing against Version("Treq", ...) that they've constructed themselves.
  • This (title-case) is what Twisted does, so it's consistent with that

What is your concern specifically? What do you want to make the capitalization consistent with?

@dmurvihill
Copy link
Author

dmurvihill commented Mar 28, 2017

Hi @glyph, you make a persuasive argument for closing this change. My only concern is that the name 'Treq' in _version.py is inconsistent with the name 'treq' in setup.py (and therefore in PyPi).

Perhaps a better resolution would be to provide an uppercase package name 'Treq'. This would resolve the inconsistency, and as a bonus it would also be consistent caps with the name 'Twisted'.

EDIT: This appears to be referenced in #181.

@glyph
Copy link
Member

glyph commented Mar 29, 2017

I am going to leave it up to @markrwilliams, who has signed up to be the release manager :)

@markrwilliams
Copy link
Member

As the release manager I side with @dmurvihill and thus prefer treq.

Here's why:

It's a proper noun, so capitalization is appropriate

Yes, but the PyPI name is treq, and I prefer consistency with that to adherence to the rules of English.

Compatibility is a concern, because others might be testing against Version("Treq", ...) that they've constructed themselves.

incremental does the right thing because of a similar issue in another project:

>>> import incremental as I
>>> I.Version('Treq', 1, 1, 1)
Version('Treq', 1, 1, 1)
>>> I.Version('Treq', 1, 1, 1) == I.Version('treq', 1, 1, 1)
True

This (title-case) is what Twisted does, so it's consistent with that

Agreed, but I think it's more confusing to have PyPI and the project name disagree than it is to have the project name disagree with other Twisted projects.

@dmurvihill Can you resolve the conflicts, and also the capitalization throughout treq._version? The best way would be to recreate it with rm src/treq/_version.py && python -m incremental.update --create treq && python -m incremental.update treq --newversion 17.3.1

This commit changes the name 'Treq' in _version.py to 'treq'. This makes the Version capitalization consistent with the caps throughout the package, and makes it possible to use __version__.local() to generate a local version number with incremental.
@dmurvihill
Copy link
Author

Should be up to date now.

@twm
Copy link
Contributor

twm commented Apr 21, 2017

I think our IRC discussion a few weeks ago was r+? Is there anything preventing merge?

@glyph
Copy link
Member

glyph commented Apr 21, 2017

@twm - perhaps I should give you commit :)

@twm
Copy link
Contributor

twm commented Apr 22, 2017

@glyph Perhaps you should. :)

@glyph glyph merged commit 9abf012 into twisted:master Apr 22, 2017
@glyph
Copy link
Member

glyph commented Apr 22, 2017

Deferring to @markrwilliams here, since he is the release manager.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants