Re: [jira] Commented: (BUILDR-222) Support Git as a version control system

[Alexis Midon]

On Tue, Dec 9, 2008 at 11:55 AM, Assaf Arkin (JIRA) <jira@...> wrote:

>
>    [
> https://issues.apache.org/jira/browse/BUILDR-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12654926#action_12654926]
>
> Assaf Arkin commented on BUILDR-222:
> ------------------------------------
>
> Awesome. A few comments:
>
> When you call Git.commit, it supers into a "common" commit (but only for
> SVN and Git, not Hg) which then subs through a bin method to perform the
> actual command, which doesn't mean the same thing in all VCS. In terms of
> readability, there's more code to deal with than if the commit method was
> never shared.

ok, I will push down the super implementation.


>
> Git.commit is sometimes commit, but sometimes it's pushes, which happens to
> be closer in spirit to an SVN commit, which just means the method name is
> not self-describing.


any suggestions?



>
> Vcs::Base.execute requires 8 lines (!) to deal with the :nofail option,
> which in turn is only used once by Git's uncommitted_files method. Everybody
> gets to share that code path, though. Much simpler is to have
> uncommited_files deal with its own personal issues, changing its first line
> to:
>
> status = `git status`
>

Actually I evaluated this option and found it not so great for testing. I
can't see how to mock this shell call. I'm a bit uncomfortable with that.


>
> Git.applies_to? should look for .git (or .git/config) in the current
> directory instead of running git branch. As a result of this change, you'll
> only be able to make a release from a buildfile at the root of the project,
> a good restriction to have.
>

Same issue for me here: testing/mocking. And the restriction you mentioned
will not be valid  for Svn.



>
> Are both vcs and guess_vcs method necessary?


Actually not. I will merge them.

>
> > Support Git as a version control system
> > ---------------------------------------
> >
> >                 Key: BUILDR-222
> >                 URL: https://issues.apache.org/jira/browse/BUILDR-222
> >             Project: Buildr
> >          Issue Type: Improvement
> >          Components: Core features
> >            Reporter: Alexis Midon
> >         Attachments: BUILDR-222-0001-git-support-for-release.txt
> >
> >
> > The Release task uses Subversion to check for uncommitted files and tag
> the source repository.
> > Of course this fails when your project does not use Subversion but Git
> for instance.
> > The improvement could be to provide a VCS abstraction with 2
> implementations: Subversion and Git.
> > A method could instantiate the proper implementation.
>
> --
> This message is automatically generated by JIRA.
> -
> You can reply to this email to add a comment to the issue online.
>
>

[Alexis Midon]

Alexis


On Tue, Dec 9, 2008 at 1:39 PM, Alexis Midon <alexismidon@...> wrote:

>
> On Tue, Dec 9, 2008 at 11:55 AM, Assaf Arkin (JIRA) <jira@...>wrote:
>
>>
>>    [
>> https://issues.apache.org/jira/browse/BUILDR-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12654926#action_12654926]
>>
>> Assaf Arkin commented on BUILDR-222:
>> ------------------------------------
>>
>> Awesome. A few comments:
>>
>> When you call Git.commit, it supers into a "common" commit (but only for
>> SVN and Git, not Hg) which then subs through a bin method to perform the
>> actual command, which doesn't mean the same thing in all VCS. In terms of
>> readability, there's more code to deal with than if the commit method was
>> never shared.
>
>
> ok, I will push down the super implementation.
>
>
>>
>> Git.commit is sometimes commit, but sometimes it's pushes, which happens
>> to be closer in spirit to an SVN commit, which just means the method name is
>> not self-describing.
>
>
> any suggestions?
>
>
>
>>
>> Vcs::Base.execute requires 8 lines (!) to deal with the :nofail option,
>> which in turn is only used once by Git's uncommitted_files method. Everybody
>> gets to share that code path, though. Much simpler is to have
>> uncommited_files deal with its own personal issues, changing its first line
>> to:
>>
>> status = `git status`
>>
>
> Actually I evaluated this option and found it not so great for testing. I
> can't see how to mock this shell call. I'm a bit uncomfortable with that.
>
>
>>
>> Git.applies_to? should look for .git (or .git/config) in the current
>> directory instead of running git branch. As a result of this change, you'll
>> only be able to make a release from a buildfile at the root of the project,
>> a good restriction to have.
>>
>
> Same issue for me here: testing/mocking. And the restriction you mentioned
> will not be valid  for Svn.
>
>

read "the restriction you mentioned *will be* valid  for Svn."

>
>
>
>>
>> Are both vcs and guess_vcs method necessary?
>
>
> Actually not. I will merge them.
>
>
>>
>> > Support Git as a version control system
>> > ---------------------------------------
>> >
>> >                 Key: BUILDR-222
>> >                 URL: https://issues.apache.org/jira/browse/BUILDR-222
>> >             Project: Buildr
>> >          Issue Type: Improvement
>> >          Components: Core features
>> >            Reporter: Alexis Midon
>> >         Attachments: BUILDR-222-0001-git-support-for-release.txt
>> >
>> >
>> > The Release task uses Subversion to check for uncommitted files and tag
>> the source repository.
>> > Of course this fails when your project does not use Subversion but Git
>> for instance.
>> > The improvement could be to provide a VCS abstraction with 2
>> implementations: Subversion and Git.
>> > A method could instantiate the proper implementation.
>>
>> --
>> This message is automatically generated by JIRA.
>> -
>> You can reply to this email to add a comment to the issue online.
>>
>>
>

[Assaf Arkin]

On Tue, Dec 9, 2008 at 1:39 PM, Alexis Midon <alexismidon@...> wrote:

> On Tue, Dec 9, 2008 at 11:55 AM, Assaf Arkin (JIRA) <jira@...> wrote:
>
>>
>>    [
>> https://issues.apache.org/jira/browse/BUILDR-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12654926#action_12654926]
>>
>> Assaf Arkin commented on BUILDR-222:
>> ------------------------------------
>>
>> Awesome. A few comments:
>>
>> When you call Git.commit, it supers into a "common" commit (but only for
>> SVN and Git, not Hg) which then subs through a bin method to perform the
>> actual command, which doesn't mean the same thing in all VCS. In terms of
>> readability, there's more code to deal with than if the commit method was
>> never shared.
>
>
> ok, I will push down the super implementation.
>
>
>>
>> Git.commit is sometimes commit, but sometimes it's pushes, which happens to
>> be closer in spirit to an SVN commit, which just means the method name is
>> not self-describing.
>
>
> any suggestions?
>
>
>
>>
>> Vcs::Base.execute requires 8 lines (!) to deal with the :nofail option,
>> which in turn is only used once by Git's uncommitted_files method. Everybody
>> gets to share that code path, though. Much simpler is to have
>> uncommited_files deal with its own personal issues, changing its first line
>> to:
>>
>> status = `git status`
>>
>
> Actually I evaluated this option and found it not so great for testing. I
> can't see how to mock this shell call. I'm a bit uncomfortable with that.

You can define a method specifically for stubbing, that's different
from overloading execute. Other options:

-  open('|git status').read
-  mock command line executable, point to it by setting ENV['PATH'] = Dir.pwd

>> Git.applies_to? should look for .git (or .git/config) in the current
>> directory instead of running git branch. As a result of this change, you'll
>> only be able to make a release from a buildfile at the root of the project,
>> a good restriction to have.
>>
>
> Same issue for me here: testing/mocking. And the restriction you mentioned
> will not be valid  for Svn.

To mock you just need to create a .git/config file in the current
working directory. You can do anything you want there, it's a
temporary directory created only to run the test

If you check that the URL returned from svn info ends in /trunk or
/branch/name, you're limiting release to the project root. svn info on
a sub-directory will return a different URL.

Assaf

>
>
>
>>
>> Are both vcs and guess_vcs method necessary?
>
>
> Actually not. I will merge them.
>
>
>>
>> > Support Git as a version control system
>> > ---------------------------------------
>> >
>> >                 Key: BUILDR-222
>> >                 URL: https://issues.apache.org/jira/browse/BUILDR-222
>> >             Project: Buildr
>> >          Issue Type: Improvement
>> >          Components: Core features
>> >            Reporter: Alexis Midon
>> >         Attachments: BUILDR-222-0001-git-support-for-release.txt
>> >
>> >
>> > The Release task uses Subversion to check for uncommitted files and tag
>> the source repository.
>> > Of course this fails when your project does not use Subversion but Git
>> for instance.
>> > The improvement could be to provide a VCS abstraction with 2
>> implementations: Subversion and Git.
>> > A method could instantiate the proper implementation.
>>
>> --
>> This message is automatically generated by JIRA.
>> -
>> You can reply to this email to add a comment to the issue online.
>>
>>
>

[Alexis Midon]

Thanks Assaf.
I'm improving the patch.

Alexis


On Tue, Dec 9, 2008 at 1:57 PM, Assaf Arkin <arkin@...> wrote:

> On Tue, Dec 9, 2008 at 1:39 PM, Alexis Midon <alexismidon@...>
> wrote:
> > On Tue, Dec 9, 2008 at 11:55 AM, Assaf Arkin (JIRA) <jira@...>
> wrote:
> >
> >>
> >>    [
> >>
> https://issues.apache.org/jira/browse/BUILDR-222?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12654926#action_12654926
> ]
> >>
> >> Assaf Arkin commented on BUILDR-222:
> >> ------------------------------------
> >>
> >> Awesome. A few comments:
> >>
> >> When you call Git.commit, it supers into a "common" commit (but only for
> >> SVN and Git, not Hg) which then subs through a bin method to perform the
> >> actual command, which doesn't mean the same thing in all VCS. In terms
> of
> >> readability, there's more code to deal with than if the commit method
> was
> >> never shared.
> >
> >
> > ok, I will push down the super implementation.
> >
> >
> >>
> >> Git.commit is sometimes commit, but sometimes it's pushes, which happens
> to
> >> be closer in spirit to an SVN commit, which just means the method name
> is
> >> not self-describing.
> >
> >
> > any suggestions?
> >
> >
> >
> >>
> >> Vcs::Base.execute requires 8 lines (!) to deal with the :nofail option,
> >> which in turn is only used once by Git's uncommitted_files method.
> Everybody
> >> gets to share that code path, though. Much simpler is to have
> >> uncommited_files deal with its own personal issues, changing its first
> line
> >> to:
> >>
> >> status = `git status`
> >>
> >
> > Actually I evaluated this option and found it not so great for testing. I
> > can't see how to mock this shell call. I'm a bit uncomfortable with that.
>
> You can define a method specifically for stubbing, that's different
> from overloading execute. Other options:
>
> -  open('|git status').read
> -  mock command line executable, point to it by setting ENV['PATH'] =
> Dir.pwd
>
> >> Git.applies_to? should look for .git (or .git/config) in the current
> >> directory instead of running git branch. As a result of this change,
> you'll
> >> only be able to make a release from a buildfile at the root of the
> project,
> >> a good restriction to have.
> >>
> >
> > Same issue for me here: testing/mocking. And the restriction you
> mentioned
> > will not be valid  for Svn.
>
> To mock you just need to create a .git/config file in the current
> working directory. You can do anything you want there, it's a
> temporary directory created only to run the test
>
> If you check that the URL returned from svn info ends in /trunk or
> /branch/name, you're limiting release to the project root. svn info on
> a sub-directory will return a different URL.
>
> Assaf
>
> >
> >
> >
> >>
> >> Are both vcs and guess_vcs method necessary?
> >
> >
> > Actually not. I will merge them.
> >
> >
> >>
> >> > Support Git as a version control system
> >> > ---------------------------------------
> >> >
> >> >                 Key: BUILDR-222
> >> >                 URL: https://issues.apache.org/jira/browse/BUILDR-222
> >> >             Project: Buildr
> >> >          Issue Type: Improvement
> >> >          Components: Core features
> >> >            Reporter: Alexis Midon
> >> >         Attachments: BUILDR-222-0001-git-support-for-release.txt
> >> >
> >> >
> >> > The Release task uses Subversion to check for uncommitted files and
> tag
> >> the source repository.
> >> > Of course this fails when your project does not use Subversion but Git
> >> for instance.
> >> > The improvement could be to provide a VCS abstraction with 2
> >> implementations: Subversion and Git.
> >> > A method could instantiate the proper implementation.
> >>
> >> --
> >> This message is automatically generated by JIRA.
> >> -
> >> You can reply to this email to add a comment to the issue online.
> >>
> >>
> >
>