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

Some minor code adjustments plus a CLI #134

Merged
merged 2 commits into from
Sep 25, 2016
Merged

Some minor code adjustments plus a CLI #134

merged 2 commits into from
Sep 25, 2016

Conversation

nir0s
Copy link
Collaborator

@nir0s nir0s commented Jul 23, 2016

No description provided.

@nir0s
Copy link
Collaborator Author

nir0s commented Jul 28, 2016

There's a problem with using argparse here. It's not supported by default in 2.6 (which we should still support due to the % of companies using it).

@asottile
Copy link
Contributor

asottile commented Aug 3, 2016

For argparse you can do conditional requirements (such as this example)

@andy-maier
Copy link
Collaborator

From looking at the code, it seems that test case TestCliassumes that the distrocommand outputs JSON format, but it does that only when invoked with the -j option.

@andy-maier
Copy link
Collaborator

What do people think about an lsb_release compatible cli for distro?

@nir0s
Copy link
Collaborator Author

nir0s commented Aug 15, 2016

In what sense?

@nir0s
Copy link
Collaborator Author

nir0s commented Aug 15, 2016

As suggested by @asottile , do we agree that argparse should be conditional and that if someone who uses 2.6 doesn't have argparse it becomes a dependency?

@MartijnBraam
Copy link
Collaborator

I think that's the better option, I personally find that argparse produces a way better command line tool than optparse.

The oldest python version I used is 2.7 and I don't have any experience with optional dependencies so I don't know the pitfalls but it seems like a good solution.

@asottile
Copy link
Contributor

Another option would be to drop 2.6 support (which has been end of lifed for several years).

@nir0s nir0s force-pushed the add-cli branch 2 times, most recently from 69c0537 to 4c19e7f Compare August 16, 2016 10:43
@nir0s
Copy link
Collaborator Author

nir0s commented Aug 16, 2016

Many companies still use 2.6. See the PR. I added argparse as a conditional requirement.
Also, added an entry point so that you can simply run distro instead of python -m distro.

@nir0s
Copy link
Collaborator Author

nir0s commented Aug 28, 2016

If we're going to release 1.0.0, I think we need this in..

* The CLI allows to printout either json formatted info or human readable info.
* Add a `distro` entry point so that you can run `distro` directly.
@nir0s nir0s merged commit 08ea314 into master Sep 25, 2016
@nir0s nir0s deleted the add-cli branch September 25, 2016 15:10
@nir0s
Copy link
Collaborator Author

nir0s commented Sep 27, 2016

solves #133

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.

4 participants