-
Notifications
You must be signed in to change notification settings - Fork 14
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
Split Constantly out into its own project. #1
Conversation
|
Hah, you beat me to the punch. :) Found this just as I was going to push my branch. The tests also pass in python3.2, python3.3, and python 3.4, so perhaps we might as well add envs for that? |
| Constantly | ||
| ========== | ||
|
|
||
| Constants in Python! |
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.
Maybe the docstring from constants.py would also work as a description here? "Library that provides symbolic constant support, including collections and constants with text, numeric, and bit flag values."
|
I don't have any py3 on my machine, so I really can't test that :(. Getting constantly onto travis is a good idea so that I can mess with it - cc @glyph ? |
|
Nope, you're right, unittest2 doesn't seem to work on python3.3, apologies. I set up my tox environment wrong. All the tox tests look good. Up to you if you want to update the description. Otherwise LGTM. |
|
there is https://pypi.python.org/pypi/unittest2py3k ? |
|
I think we can deal with py3 support later (ie. in another PR). |
|
Sure. :) All the tox tests pass for me. Thanks for working on this and getting it done almost as soon as the ticket was filed! I think this is good to merge. |
| nose | ||
| unittest2 | ||
| commands = | ||
| nosetests --with-coverage --cover-branches --cover-min-percentage=100 --cover-package=constantly |
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.
Please don't introduce a nose dependency here.
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.
So, use python -m unittest and coverage directly, then?
|
I think that's all the problems addressed! I also moved it to versioneer as it was easier than unscrewing the boilerplate setup.py code. |
| @@ -0,0 +1,9 @@ | |||
| # file GENERATED by distutils, do NOT edit | |||
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.
Surely generated files don't belong in version control?
| Jean-Paul Calderone | ||
| Richard Wall | ||
| Wilfredo Sánchez | ||
| HawkOwl |
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.
missing newline at the end of this file
| @@ -997,7 +997,7 @@ def test_orderedFlagConstants_ge(self): | |||
|
|
|||
| def test_orderedDifferentConstants_lt(self): | |||
| """ | |||
| L{twisted.python.constants._Constant.__lt__} returns C{NotImplemented} | |||
| L{constantly._Constant.__lt__} returns C{NotImplemented} | |||
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 isn't actually the right name. Private implementation details like _Constant didn't get imported and re-exposed in constantly. This should be constantly.constants._Constant.__lt__.
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 pattern repeats below.
|
Thanks. That's all from me for now. |
|
Finally got back to this. I read through the tests and made sure that the links were right. There's some pydoctors errors, but that's because PyDoctor has problems with the weird subclassing situation that Constantly does. I think it's in somewhat of a shape now. |
|
@cyli I test unittest2 on python 3.3 before every release. When you say it doesn't work, can you provide a trace, or file a ticket at https://github.com/testing-cabal/unittest-ext ? |
|
@rbtcollins My memory for these things is basically about two weeks, so I have no idea any more. :) I'll try it again later though. |
|
Since this has kind of sat for long enough, I'm going to merge it unless anyone has any objections. |
|
LGTM. |
Split Constantly out into its own project.
This does the following: