-
Notifications
You must be signed in to change notification settings - Fork 119
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
updating for versioneer #145
Conversation
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.
The install stuff isn't vital so merge if you want. If you want to make sure everything is behaving, I'd double check this RunTests class business.
print('removing {}'.format(d)) | ||
shutil.rmtree(d) | ||
|
||
|
||
# thank you https://stormpath.com/blog/building-simple-cli-interfaces-in-python | ||
class RunTests(Command): | ||
"""Run all tests.""" |
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.
Is this class redundant now that you're using "cmdclass": versioneer.get_cmdclass(),
below? Looking at https://stormpath.com/blog/building-simple-cli-interfaces-in-python again, I think it needs to be setup something like https://github.com/znicholls/netcdf-scm/blob/master/setup.py for this to actually do anything.
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.
Yes, it is. I wasn't sure what exactly to do with it, since I didn't add it (I believe you did, right?).
In any case, I can mirror your netcdf-scm style or we can just remove it. I always run pytest tests
and not python setup.py test
. Do you have a preference?
Pep440, toss a coin
Command class, I think copy netcdfscm. I think you have to have the command
class in there and this seems to be the way to make it work..
…On Thu, 29 Nov 2018 at 5:09 pm, Matthew Gidden ***@***.***> wrote:
***@***.**** commented on this pull request.
------------------------------
In setup.py
<#145 (comment)>:
> -
- def finalize_options(self):
- install.finalize_options(self)
-
- def run(self):
- install.run(self)
- dirs = [
- 'pyam.egg-info',
- 'build',
- ]
- for d in dirs:
- if os.path.exists(d):
- print('removing {}'.format(d))
- shutil.rmtree(d)
-
-
# thank you https://stormpath.com/blog/building-simple-cli-interfaces-in-python
class RunTests(Command):
"""Run all tests."""
Yes, it is. I wasn't sure what exactly to do with it, since I didn't add
it (I believe you did, right?).
In any case, I can mirror your netcdf-scm style or we can just remove it.
I always run pytest tests and not python setup.py test. Do you have a
preference?
—
You are receiving this because your review was requested.
Reply to this email directly, view it on GitHub
<#145 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AWh-m-gXQoh9cQZZPr9jlkNHlnH3aDxsks5uz6RUgaJpZM4Y1gPT>
.
|
ok should be g2g @znicholls |
lgtm |
Please confirm that this PR has done the following:
Adding to RELEASE_NOTES.md (remove section after adding to RELEASE_NOTES.md)
Please add a single line in the release notes similar to the following:
Description of PR
This PR uses
versioneer
to automatically generate appropriate version numbers. If on a tag, it will provide the current tag as the version (for packaging). If on a certain commit, it will name the commit like so: