Skip to content

Make setup py gets version from file#47

Closed
usta wants to merge 1 commit intoedwardgeorge:masterfrom
usta:add_solo_version_place_for_all
Closed

Make setup py gets version from file#47
usta wants to merge 1 commit intoedwardgeorge:masterfrom
usta:add_solo_version_place_for_all

Conversation

@usta
Copy link
Contributor

@usta usta commented Jan 23, 2019

I believe with this way it will be much suitable for version bumps.
This PR also fixes a small bug about a typo in MANIFEST.in
fixes #44

I believe with this way it will be much suitable for version bumps.
This PR also fixes a small bug about a typo in MANIFEST.in
@timabbott
Copy link
Collaborator

@eeshangarg I assume you'll review (are you subscribed to notifications for this repo such that I don't need to mention you?)

Copy link
Contributor

@eeshangarg eeshangarg left a comment

Choose a reason for hiding this comment

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

@usta: Thanks for working on this! I tested this and it works.

Now that I think about it, the __version__ value is kinda useless. The only purpose it serves is printing an error message for when the script isn't given enough command-line arguments. For that, we don't really need the version number. And since the package is never really imported (it only has an executable), I can't imagine a scenario where a user may go import clonevirtualenv; clonevirtualenv.__version__, they can just use pip freeze if they want to find out the version. Is there a reason why a __version__ value might be useful for an executable-only package that I am not aware of? If not, we could honestly just get rid of it and that would make everything simpler.

Even some popular Python packages have a history of using "hacks" to store version numbers globally in an external file, so it isn't a big deal either way, but still worth a short discussion. :) Thanks!

@@ -1,2 +1,2 @@
include README
include README.md
include LICENSE
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please split this one out into a separate commit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could you please split this one out into a separate commit?

Sure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@eeshangarg #50 is ready

@eeshangarg
Copy link
Contributor

@timabbott: yes, I just subscribed to alerts for this repo, so you don't have to explicitly mention me in issues/PRs anymore! Thanks! :)

@usta
Copy link
Contributor Author

usta commented Jan 27, 2019

@usta: Thanks for working on this! I tested this and it works.

Now that I think about it, the __version__ value is kinda useless. The only purpose it serves is printing an error message for when the script isn't given enough command-line arguments. For that, we don't really need the version number. And since the package is never really imported (it only has an executable), I can't imagine a scenario where a user may go import clonevirtualenv; clonevirtualenv.__version__, they can just use pip freeze if they want to find out the version. Is there a reason why a __version__ value might be useful for an executable-only package that I am not aware of? If not, we could honestly just get rid of it and that would make everything simpler.

Even some popular Python packages have a history of using "hacks" to store version numbers globally in an external file, so it isn't a big deal either way, but still worth a short discussion. :) Thanks!

If you ask my opinion it will be much better we just have the version number in setup.py, not in other files.
If you accept I will prepare a striped version of version :)

@usta
Copy link
Contributor Author

usta commented Jan 27, 2019

@eeshangarg @timabbott also I have stripped down the version info from script file so we just follow version in setup.py with : #51

@eeshangarg
Copy link
Contributor

@usta: Cool sounds good, I just reviewed your other PRs! :) Also, could you please close this PR now that we've decided to go the other way? :)

@usta
Copy link
Contributor Author

usta commented Feb 9, 2019

We decided to make this PR canceled for benefits of other PRs

@usta usta closed this Feb 9, 2019
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.

version numbers must be collected into one place

3 participants