Ticket #701 (closed enhancement: wontfix)

Opened 11 years ago

Last modified 11 years ago

git syncer should be aware of "repository" and "refspec" options

Reported by: ra Owned by:
Priority: Sometime Milestone:
Component: clients/paludis Version: 0.32.4
Keywords: Cc:
Blocked By: Blocking: 731
Distribution: Gentoo

Description

attached is a patch to do so

Attachments

dogit.patch Download (964 bytes) - added by ra 11 years ago.
add --git-repository and --git-refspec
0001-add-git-repository-and-git-refspec-to-git-syncer.patch Download (1.4 KB) - added by ra 11 years ago.
add --git-repository and --git-refspec to git syncer

Change History

Changed 11 years ago by ra

add --git-repository and --git-refspec

comment:1 Changed 11 years ago by ra

if not using symlinks for the (git*) syncers (see  http://trac.pioto.org/paludis/ticket/700) the patch should be also applied to dogit+http, dogit+ssh, dogit+file, dogit+https and dogit+rsync.

comment:2 follow-up: ↓ 3 Changed 11 years ago by dleverton

A few comments:

  • "repository" is probably better as "remote" IMO
  • You should replace all references to "origin", not just that one
  • Does "${GIT_REFSPEC}" work properly if the user doesn't set it, or does git fail when passed an empty string as an argument? You can probably just drop the quotes here
  • Would "refspec" be easier to understand as "branch", and if so, can you make it work for the initial clone too?
  • Is this going to do horrible messy things if the user ever changes the options, such as trying to merge two branches together or the like?

Also, please use git format-patch to make patches, if at all possible.

Changed 11 years ago by ra

add --git-repository and --git-refspec to git syncer

comment:3 in reply to: ↑ 2 ; follow-up: ↓ 4 Changed 11 years ago by ra

Replying to dleverton:

  • "repository" is probably better as "remote" IMO

the term "repository" is being used in the patch. i dont really get what you mean, sorry ):

  • You should replace all references to "origin", not just that one

which other references do you refer to?

  • Does "${GIT_REFSPEC}" work properly if the user doesn't set it, or does git fail when passed an empty string as an argument? You can probably just drop the quotes here

yes, it does work if GIT_REFSPEC is undefined.

  • Would "refspec" be easier to understand as "branch", and if so, can you make it work for the initial clone too?

man git-pull refers to it as "refspec" so i thought to leave it the same name.

  • Is this going to do horrible messy things if the user ever changes the options, such as trying to merge two branches together or the like?

i currently use it for syncing the funtoo tree. changing branches and renaming them didnt cause any problems.

Also, please use git format-patch to make patches, if at all possible.

done.

comment:4 in reply to: ↑ 3 ; follow-up: ↓ 8 Changed 11 years ago by dleverton

Replying to ra:

Replying to dleverton:

  • "repository" is probably better as "remote" IMO

the term "repository" is being used in the patch. i dont really get what you mean, sorry ):

Yes, I mean the patch should probably call it "remote" instead of "repository".

  • You should replace all references to "origin", not just that one

which other references do you refer to?

Everywhere else in the syncer that the word "origin" occurs.

  • Would "refspec" be easier to understand as "branch", and if so, can you make it work for the initial clone too?

man git-pull refers to it as "refspec" so i thought to leave it the same name.

*shrug* Up to you, then.

  • Does "${GIT_REFSPEC}" work properly if the user doesn't set it, or does git fail when passed an empty string as an argument? You can probably just drop the quotes here

yes, it does work if GIT_REFSPEC is undefined.

  • Is this going to do horrible messy things if the user ever changes the options, such as trying to merge two branches together or the like?

i currently use it for syncing the funtoo tree. changing branches and renaming them didnt cause any problems.

Also, please use git format-patch to make patches, if at all possible.

done.

Good, good and good.

comment:5 Changed 11 years ago by ciaranm

  • Blocking set to 714

Could we get an updated patch for this please? Would like to have it in for 0.36.

comment:6 Changed 11 years ago by ra

The patch still applies cleanly. I should mention that i don't use it anymore so I haven't done any further testing.

comment:7 Changed 11 years ago by ciaranm

dleverton: was there anything in the most recent patch here you weren't happy with?

comment:8 in reply to: ↑ 4 Changed 11 years ago by dleverton

Replying to ciaranm:

dleverton: was there anything in the most recent patch here you weren't happy with?

I still don't see an answer to these bits:

Replying to dleverton:

Replying to ra:

Replying to dleverton:

  • "repository" is probably better as "remote" IMO

the term "repository" is being used in the patch. i dont really get what you mean, sorry ):

Yes, I mean the patch should probably call it "remote" instead of "repository".

  • You should replace all references to "origin", not just that one

which other references do you refer to?

Everywhere else in the syncer that the word "origin" occurs.

comment:9 Changed 11 years ago by ciaranm

  • Blocking changed from 714 to 731

Moving the target forward until we get an updated patch then.

comment:10 Changed 11 years ago by ciaranm

  • Status changed from new to closed
  • Resolution set to wontfix

So far as I can see, the --branch and --reset options I've just committed supersede this.

Note: See TracTickets for help on using tickets.