search

mroth / scmpuff

:1234: Numeric file shortcuts for common git commands
https://mroth.github.io/scmpuff/
MIT License
383 stars 22 forks source link

Fix git init issue #37

Closed zommerfelds closed 6 years ago

zommerfelds commented 6 years ago

Attempt to fix #36

mroth commented 6 years ago

Awesome! Thanks for taking this on.

zommerfelds commented 6 years ago

Does git ever localize this output? (separate issue maybe)

On Wed, Mar 21, 2018, 12:52 Matthew Rothenberg notifications@github.com wrote:

@mroth commented on this pull request.

In commands/status/process_test.go https://github.com/mroth/scmpuff/pull/37#discussion_r176156916:

@@ -238,6 +238,10 @@ var testCasesExtractBranch = []struct { []byte("## Initial commit on master"),

Ah yeah, looks like it was changed in git/git@4ddb135 https://github.com/git/git/commit/4ddb1354e8d5daf5671d3d451a67d2d1e82d9b49

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mroth/scmpuff/pull/37#discussion_r176156916, or mute the thread https://github.com/notifications/unsubscribe-auth/ABM8TmT2mxbji9CZWUdsL7i7ovYRcp-Sks5tgoUdgaJpZM4Sy779 .

zommerfelds commented 6 years ago

Is there anything left to be done before we can merge this?

mroth commented 6 years ago

Is there anything left to be done before we can merge this?

Sorry for the delay, I was out of the country.

The one thing I was thinking is that this is a case we should probably have an integration test for as well, so we automatically catch it if the language changes again or similar.

zommerfelds commented 6 years ago

Good point. But actually, there is already a test case for it:

  Scenario: Banner shows no changes when in an unchanged git repo
    Given I am in a git repository
    When I successfully run `scmpuff status`
    Then the output should contain "No changes (working directory clean)"

So, currently the tests are not even passing on master with the newer git version. I created a PR below just to demonstrate this on CI: https://github.com/mroth/scmpuff/issues/36

mroth commented 6 years ago

Oh great, I somehow forgot that integration test already existed. This should be ready to be merged then. Thanks so much for your contribution @zommerfelds !