frostming / legit

Git for Humans, Inspired by GitHub for Mac™.
https://frostming.github.io/legit
BSD 3-Clause "New" or "Revised" License
5.7k stars 218 forks source link

Modernize CLI #233

Closed grahamu closed 6 years ago

grahamu commented 6 years ago

Add --verbose and --fake options. Implement test suite and activate CI testing. Implement command line argument handling and configuration with click.

Why click

Additional Changes and Improvements

Deprecations

Reviewer Notes

Demonstration screencap: https://www.useloom.com/share/d876b8c1e34c4fd2aaceeac034567d5b

grahamu commented 6 years ago

@kennethreitz I consider this PR a work in progress for a package I love and use every day.

Tests are needed to prove (for instance) that the --fake option does not invoke git. If you're happy with the direction this PR is going, I'll add some tests so there's at least something executed in each version of supported Python in the test suite. Thanks for your consideration.

kennethreitz commented 6 years ago

Feel free to merge whenever you feel it's appropriate :)

grahamu commented 6 years ago

Roger, wilco. Thanks @kennethreitz!

grahamu commented 6 years ago

@kennethreitz is Travis enabled for this project? I'd sure like to see Travis happy with this PR before merging with develop, and I have a feeling some tests may fail in Travis' environment.

https://travis-ci.org/kennethreitz/legit

Same question for coveralls. Thanks!

Aside from CI testing this PR is done. The screencap video has been updated.

kennethreitz commented 6 years ago

activated!

kennethreitz commented 6 years ago

https://travis-ci.org/kennethreitz/legit/builds/351954964

grahamu commented 6 years ago

@kennethreitz this PR is ready for review with updated description. Tests provide 73% coverage on Python 2.7, 3.4, 3.5, and 3.6 as seen at https://travis-ci.org/kennethreitz/legit. There are a few deprecations and minor interface changes. Basically I tried to emulate pipenv invocation patterns and help style. The PR description is now up-to-date, worth re-reading to ensure you're in agreement with these design changes. Cheers!

grahamu commented 6 years ago

I've been dogfooding legit during development. Works well for me, but obviously could use more real-world testing in varied git scenarios. Regression tests welcome!

grahamu commented 6 years ago

Hi @weakish you've been involved quite a bit lately, comments welcome!

weakish commented 6 years ago

@grahamu Have done a rough review of all the commits.

Also tested several commits on my machine (in a test repository by hand.)

Everything seems O.K.

kennethreitz commented 6 years ago

okay I have some thoughts about 1.0.0

kennethreitz commented 6 years ago

mostly — this bug

I want us to release a patched version of the dependency causing this (there's a version on github which fixes this) and depend on that instead.

kennethreitz commented 6 years ago

Also, instead of --edit, I think --configure or --config would be more appropriate.

kennethreitz commented 6 years ago

@weakish the help.replace pattern is a click-workaround done by pipenv. I approve :)

weakish commented 6 years ago

@kennethreitz the packed-refs bug still exist? cd14865 did not fix it?

kennethreitz commented 6 years ago

oh yay! I'm using an old version :)

grahamu commented 6 years ago

@weakish re Editor/IDE/OS specific files are recommended to specified in global git ignore file, not project level git ignore file. Good point... I've removed several patterns from .gitignore.

@weakish re typo: "still list" -> "still lists". Good catch. That sentence now starts, "If legit branches still shows it as published,"

@weakish re help.replace smells hacky. I agree. I spent considerable time trying to bend click to our wishes here without success, and eventually decided emulating pipenv was a reasonable solution. It also suffers from pipenv and pipenv --help showing different results. However, I was able to get all main help invocations to produce the same formatted result, so at least there's no disconnect.

@kennethreitz changed --edit to --config, good call.

@kennethreitz One more addition... deprecated legit commands are now included in the --uninstall option, i.e. "graft". See https://github.com/kennethreitz/legit/pull/233/files#diff-864e74797332274ad08772c50493fe0fR285.

I believe all reviewer commentary is addressed. This last utility method refactor cleans up both cli.py and scm.py into utils.py.

kennethreitz commented 6 years ago

✨🍰✨

kennethreitz commented 6 years ago

Let's get this released! Keep me in the loop, I'll make an announcement

grahamu commented 6 years ago

@kennethreitz I just released legit v1.0.0 on GitHub! I'm package maintainer for around 30 libraries (mainly Pinax), but don't have publishing rights for legit. Happy to handle that given perms, otherwise publish at your leisure. Thanks!

grahamu commented 6 years ago

I just noticed the Travis CI build is failing. This is likely a result of basing tests on the checked out local git repo in the Travis environment, rather than on a repo "fixture".

All tests pass with flying colors locally (using my local legit repo for the tests) so I do not believe this is a blocking failure, more like a test environment failure. However the test suite really needs to be updated to operate on a git fixture.

kennethreitz commented 6 years ago

@grahamu what's your pypi username?

kennethreitz commented 6 years ago

Released on PyPi :)

grahamu commented 6 years ago

My PyPi username is grahamu

On Wed, Mar 14, 2018 at 11:47 AM, Kenneth Reitz notifications@github.com wrote:

@grahamu https://github.com/grahamu what's your pypi username?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/kennethreitz/legit/pull/233#issuecomment-373114009, or mute the thread https://github.com/notifications/unsubscribe-auth/AAH9mI5n_-dVXleZpyFu2ybAqcgKENwjks5teVewgaJpZM4Sk97L .

kennethreitz commented 6 years ago

added :)