This repository has been archived by the owner. It is now read-only.

Simplify implementation of commands that open a git repository viewer #18708

Closed
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants
Contributor

dlh commented Mar 24, 2013

No description provided.

Homebrew::log can now take a block. This makes it simpler to implemen…
…t a command that uses an external git repository browser (gitk, gitx, etc.).
Contributor

adamv commented Mar 24, 2013

Let's use this idea but implement it in a different way, such as having a git method somewhere instead of overloading the log command directly in this way.

Contributor

samueljohn commented Mar 26, 2013

Tried this, but

brew pull 18708
######################################################################## 100,0%
==> Applying patch
Applying: Homebrew::log can now take a block. This makes it simpler to implement a command that uses an external git repository browser (gitk, gitx, etc.).
==> Patch closes issue #18708
==> Patch changed:
 Library/Contributions/cmd/brew-gitk.rb | 17 +++++++++++++++++
 Library/Homebrew/cmd/log.rb            |  6 +++++-
 2 files changed, 22 insertions(+), 1 deletion(-)

/homebrew ❯ brew gitk python
Error: undefined method `log' for Homebrew:Module
Please report this bug:
    https://github.com/mxcl/homebrew/wiki/troubleshooting
/homebrew/Library/Contributions/cmd/brew-gitk.rb:15
/homebrew/Library/brew.rb:52:in `require'
/homebrew/Library/brew.rb:52:in `require?'
Contributor

mistydemeo commented Mar 26, 2013

The command needs to require 'cmd/log'. We don't require the contents of all the commands by default.

Contributor

dlh commented Mar 27, 2013

I corrected the missing require issue. The squashed commit can be found in my log-command-squashed branch.

I'm not very familar with the homebrew codebase, so I'll let someone else tackle implementing a different way to achieve this (adamv's suggestion).

Contributor

adamv commented May 2, 2013

I think both commands should import a git module, rather than having commands import commands; I'll let other maintainers decide if they think this should be pulled.

Contributor

jacknagel commented May 4, 2013

I guess I don't have any problem with requiring command files, they're just methods after all. But the logic should be pulled out into a helper method that is called by both Homebrew.log and this command, rather than overload Homebrew.log.

Refactored log command to allow for easier reuse.
* Added support for showing the log for a tapped repo, and also multiple formulae.
* Added Formula::tap_path
Contributor

adamv commented Jun 9, 2013

There is too much going on in this pull request; changes to commands in addition to the git/log refactoring.

Needs to be broken up into smaller pull requests.

@adamv adamv closed this Jun 9, 2013

@xu-cheng xu-cheng locked and limited conversation to collaborators Feb 16, 2016

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.