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

Suppress log by filtering remote refs #1804

Merged

Conversation

aeroastro
Copy link
Contributor

Summary

This patch can suppress long logs generated by git ls-remote, especially for repositories with many tags and branches.

Because the primary objective of git ls-remote used is to check the access rights to remote_url, we can omit a number of remote refs by specifying HEAD as <refs> at the end of the command.

Although the default exit code of git ls-remote is always 0 regardless of matched refs, I think specifying HEAD as <refs> is fairly simple option for Git users.

Short checklist

  • Did you run bundle exec rubocop -a to fix linter issues?
  • If relevant, did you create a test?
  • Did you confirm that the RSpec tests pass?
  • If you are fixing a bug or introducing a new feature, did you add a CHANGELOG entry?

Other Information

Ref. https://git-scm.com/docs/git-ls-remote.html

@leehambley
Copy link
Member

Thanks for the contribution @aeroastro - I have a question:

  • which versions of Git support the change you made?

First impression is that it looks OK, since it's only a repo-check and doesn't need to enumerate all objects, we could definitely pull fewer tags/heads/etc, and still have equal functionality.

@aeroastro
Copy link
Contributor Author

aeroastro commented Nov 19, 2016

Thank you for your time to look into my patch and comment.

which versions of Git support the change you made?

This change support Git with version later than or equal to 0.99.4.

The <repository> <refs>... usage was first implemented by git/git@972b6fe and officially released by 0.99.4 git/git@bf57030 on 11 August 2005. On the other hand, git ls-remote command itself was released by 0.99.2.

@leehambley
Copy link
Member

leehambley commented Nov 22, 2016

Thanks, great news. If you'd like to be credited in the CHANGELOG, please just adjust the PR, otherwise let me know, and I'll just merge it.

@mattbrictson
Copy link
Member

@aeroastro Can you add a CHANGELOG entry? Then we can merge this PR. Thanks!

@aeroastro
Copy link
Contributor Author

Sorry for the late response and thank you for the review.
I added a CHANGELOG entry. 🐱

@mattbrictson
Copy link
Member

features/deploy.feature is failing in this PR. I think at least one of the assertions is now invalid; specifically, "references in the remote repo are listed" is no longer true. Can you remove that assertion?

Unfortunately the only way to check Cucumber features is to run them locally, since we can't do it in Travis. Assuming you have a working Vagrant setup, you can run the Cucumber features using:

gem install bundler -v 1.10.5
bundle _1.10.5_ exec rake features

@aeroastro
Copy link
Contributor Author

I removed the assertion on "And references in the remote repo are listed".

Then, by executing bundle _1.10.5_ exec rake features, it was confirmed that all the scenarios and steps passed except for 1 pending scenario and 1 pending step.

@mattbrictson
Copy link
Member

Excellent work. Thanks for the PR! 🙇

@mattbrictson mattbrictson merged commit 3d9800d into capistrano:master Jan 5, 2017
@aeroastro aeroastro deleted the feature/suppress-remote-refs branch January 6, 2017 00:49
@aeroastro
Copy link
Contributor Author

Thank you! 🐱

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.

3 participants