Bug #82167 for Git-Hooks: remove optional as mandatory

Fri Dec 21 20:06:38 2012 xenoterracide [...] gmail.com - Ticket created

Subject:	remove optional as mandatory
Date:	Fri, 21 Dec 2012 19:06:27 -0600
To:	bugs-git-hooks [...] rt.cpan.org
From:	Caleb Cushing <xenoterracide [...] gmail.com>

It seems like App::gh and Jira::Client should both be optional deps (or perhaps the things requiring those should be in their own dist(s) ). I'm considering using this to help impliment some hooks, but I don't want to argue with our admins over dependencies on things like Jira::Client when we don't need it for this. -- Caleb Cushing http://xenoterracide.com

Sat Dec 22 12:12:58 2012 GNUSTAVO [...] cpan.org - Taken

Sat Dec 22 12:23:46 2012 GNUSTAVO [...] cpan.org - Correspondence added

On Fri Dec 21 20:06:38 2012, XENO wrote: Show quoted text

> It seems like App::gh and Jira::Client should both be optional deps > (or perhaps the things requiring those should be in their own dist(s) > ). I'm considering using this to help impliment some hooks, but I > don't want to argue with our admins over dependencies on things like > Jira::Client when we don't need it for this.

Hi Caleb. You're right about JIRA::Client. I just pushed a commit making it recommended instead of required: https://github.com/gnustavo/Git- Hooks/commit/dd98923fbfe698b35f95ba0c508fefde916052be. I'm working in a new plugin and I plan to make a new release in the next days. Then this change will be included. App::gh is in another league. Git::More (which is included in Git::Hooks distro) inherits from App::gh::Git, so that it must require it. Until recently Git::Hooks required App::gh::Git directly, but this module has a problem in its versioning which led me to require App::gh explicitly. It doesn't matter though, because both are provided by the same distro.

Sat Dec 22 12:23:47 2012 The RT System itself - Status changed from 'new' to 'open'

Sat Dec 22 12:24:09 2012 GNUSTAVO [...] cpan.org - Status changed from 'open' to 'resolved'

Mon Dec 31 19:53:08 2012 xenoterracide [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #82167] remove optional as mandatory
Date:	Mon, 31 Dec 2012 18:52:58 -0600
To:	bug-Git-Hooks [...] rt.cpan.org
From:	Caleb Cushing <xenoterracide [...] gmail.com>

On Sat, Dec 22, 2012 at 11:23 AM, Gustavo Leite de Mendonça Chaves via RT <bug-Git-Hooks@rt.cpan.org> wrote: Show quoted text

> App::gh is in another league. Git::More (which is included in Git::Hooks > distro) inherits from App::gh::Git, so that it must require it. Until > recently Git::Hooks required App::gh::Git directly, but this module has > a problem in its versioning which led me to require App::gh explicitly. > It doesn't matter though, because both are provided by the same distro.

what is it required for (why are you subclassing it)? maybe I could help remove it? it's just that it pulls in a lot of things that seem unnecessary for just git hooks, such as Any::Moose and LWP::Simple (or something) -- Caleb Cushing http://xenoterracide.com

Mon Dec 31 19:53:10 2012 The RT System itself - Status changed from 'resolved' to 'open'

Wed Jan 02 09:53:35 2013 GNUSTAVO [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #82167] remove optional as mandatory
Date:	Wed, 2 Jan 2013 12:52:57 -0200
To:	bug-Git-Hooks [...] rt.cpan.org
From:	Gustavo Leite de Mendonça Chaves <gnustavo [...] cpan.org>

2012/12/31 Caleb Cushing via RT <bug-Git-Hooks@rt.cpan.org> Show quoted text

> > Queue: Git-Hooks > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=82167 > > > On Sat, Dec 22, 2012 at 11:23 AM, Gustavo Leite de Mendonça Chaves via > RT <bug-Git-Hooks@rt.cpan.org> wrote:

> > App::gh is in another league. Git::More (which is included in Git::Hooks > > distro) inherits from App::gh::Git, so that it must require it. Until > > recently Git::Hooks required App::gh::Git directly, but this module has > > a problem in its versioning which led me to require App::gh explicitly. > > It doesn't matter though, because both are provided by the same distro.

> > what is it required for (why are you subclassing it)? maybe I could > help remove it? it's just that it pulls in a lot of things that seem > unnecessary for just git hooks, such as Any::Moose and LWP::Simple (or > something)

With Git::Hooks you can associate a Perl function to any Git hook. The functions receive as their first argument a Git::More object through which they can get information about the repository. When I started Git::Hooks I take a look at all the modules I could find on CPAN that in a way wrapped Git around. At the time I decided on App::gh::Git, mainly because it is the one that comes by default with the git distribution. By subclassing it in Git::More I can provide some higher level methods useful to implement hooks. And I also can cache some very commonly used information so that different hooks can grok them more efficiently. But I have to say that I'm not very satisfied with App::gh::Git as a base for Git::More. Its API is womewhat awkward and this dependency problem you highlighted is significant. I'm taking a look again at some other options, like Git::Wrapper and Git::Repository to substitute App::gh::Git for. -- Gustavo.

Thu Jan 03 16:41:12 2013 xenoterracide [...] gmail.com - Correspondence added

Subject:	Re: [rt.cpan.org #82167] remove optional as mandatory
Date:	Thu, 3 Jan 2013 15:41:02 -0600
To:	bug-Git-Hooks [...] rt.cpan.org
From:	Caleb Cushing <xenoterracide [...] gmail.com>

Show quoted text

> With Git::Hooks you can associate a Perl function to any Git hook. The > functions receive as their first argument a Git::More object through > which they can get information about the repository. > > When I started Git::Hooks I take a look at all the modules I could > find on CPAN that in a way wrapped Git around. At the time I decided > on App::gh::Git, mainly because it is the one that comes by default > with the git distribution.

it does? I wasn't aware of that. I thought it was just Git.pm (which afaik isn't on cpan) Show quoted text

> By subclassing it in Git::More I can provide some higher level methods > useful to implement hooks. And I also can cache some very commonly > used information so that different hooks can grok them more > efficiently.

Right, I just wasn't sure if there was some reason for that specific choice. Show quoted text

> But I have to say that I'm not very satisfied with App::gh::Git as a > base for Git::More. Its API is womewhat awkward and this dependency > problem you highlighted is significant. I'm taking a look again at > some other options, like Git::Wrapper and Git::Repository to > substitute App::gh::Git for.

Most of the things (which is small) that I have used Git at some level have chosen Git::Wrapper, though not sure why. I just pushed Git::Hooks::PerlCritic, if you could let me know if you're changing because I'll have to change some code internally. Also is there a better way to communicate with you than RT? -- Caleb Cushing http://xenoterracide.com

Sat Jan 05 16:36:37 2013 GNUSTAVO [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #82167] remove optional as mandatory
Date:	Sat, 5 Jan 2013 19:36:05 -0200
To:	bug-Git-Hooks [...] rt.cpan.org
From:	Gustavo Leite de Mendonça Chaves <gnustavo [...] cpan.org>

2013/1/3 Caleb Cushing via RT <bug-Git-Hooks@rt.cpan.org>: Show quoted text

> Queue: Git-Hooks > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=82167 > >

>> When I started Git::Hooks I take a look at all the modules I could >> find on CPAN that in a way wrapped Git around. At the time I decided >> on App::gh::Git, mainly because it is the one that comes by default >> with the git distribution.

> > it does? I wasn't aware of that. I thought it was just Git.pm (which > afaik isn't on cpan)

They are the same thing, as far as I know. I can't depend on Git.pm because it's not on CPAN, but App::gh::Git is. Show quoted text

>> But I have to say that I'm not very satisfied with App::gh::Git as a >> base for Git::More. Its API is womewhat awkward and this dependency >> problem you highlighted is significant. I'm taking a look again at >> some other options, like Git::Wrapper and Git::Repository to >> substitute App::gh::Git for.

> > Most of the things (which is small) that I have used Git at some > level have chosen Git::Wrapper, though not sure why.

Git::Wrapper seems to be the simplest and easiest to work with. I'm tending to go with it. Show quoted text

> I just pushed Git::Hooks::PerlCritic, if you could let me know if > you're changing because I'll have to change some code internally.

Cool! I've updated the TODO.pod file with the ideas I'll likely pursue in the near future. You can take a look at it on GitHub: https://github.com/gnustavo/Git-Hooks/blob/master/TODO.pod. Please, note that the following items will make changes that will affect your plugin: - Move plugin config options to a githooks subsection - Simplify plugin/hook integration - Find a substitute for App::gh::Git I intend to release a 1.0 version after implementing these, the Git::Hooks::SafeRewrite plugin, and port it to Windows. After that I'll be much more careful in preserving backward compatibility. Any comments you have on that list will be more than welcome. Show quoted text

> Also is there a better way to communicate with you than RT?

Sure. You can send me email at gnustavo@cpan.org. -- Gustavo.

Sun Jan 06 07:15:51 2013 GNUSTAVO [...] cpan.org - Status changed from 'open' to 'resolved'

Sat Jan 12 15:59:34 2013 GNUSTAVO [...] cpan.org - Correspondence added

Subject:	Re: [rt.cpan.org #82167] remove optional as mandatory
Date:	Sat, 12 Jan 2013 18:58:57 -0200
To:	bug-Git-Hooks [...] rt.cpan.org
From:	Gustavo Leite de Mendonça Chaves <gnustavo [...] cpan.org>

2013/1/5 Gustavo Leite de Mendonça Chaves <gnustavo@cpan.org> Show quoted text

> 2013/1/3 Caleb Cushing via RT <bug-Git-Hooks@rt.cpan.org>: >

> > Queue: Git-Hooks > > Ticket <URL: https://rt.cpan.org/Ticket/Display.html?id=82167 >

>

Show quoted text

> I've updated the TODO.pod file with the ideas I'll likely pursue in the > near future. You can take a look at it on GitHub: > https://github.com/gnustavo/Git-Hooks/blob/master/TODO.pod. Please, note > that the following items will make changes that will affect your plugin: > > - Move plugin config options to a githooks subsection > - Simplify plugin/hook integration > - Find a substitute for App::gh::Git > > Hi Caleb.

Just to let you know that I've implemented those three bullets in the recent releases of Git::Hooks. Version 0.033 should appear shortly on CPAN. Some of the changes might probably affect your PerlCritic plugin. I'm sorry for that. But the good news is that I don't have any other imcompatible change in mind for the forseable future. In the end I decided to substitute the original Git for App::gh::Git. I reimplemented everything using Git::Wrapper but get dissatisfied with the result. I think Git::Repository is more mature, but they don't bring much benefits to be worthwhile. I think keeping with the "default" wrapper is a strong motivation to keep with Git. I understood better the relation between Git and App::gh::Git. The latter is a copy of the former, maintained by the folks at GitHub for their App::gh distribution. But, as you realised, it carries some heavy dependencies that we don't need. Thank you for the hint and for your interest. -- Gustavo gnustavo@cpan.org

Sat Jan 12 15:59:35 2013 The RT System itself - Status changed from 'resolved' to 'open'

Mon Jan 14 05:50:45 2013 GNUSTAVO [...] cpan.org - Status changed from 'open' to 'resolved'

Bug #82167 for Git-Hooks: remove optional as mandatory

Preferred bug tracker