search

jonasbjurel / OPNFV-Playground

Playground for OPNFV
Apache License 2.0
3 stars 2 forks source link

DO NOT MERGE #60

Closed jonasbjurel closed 8 years ago

jonasbjurel commented 8 years ago

NOTE VERIFIED Description: Patch for patch-set handling, not needing LF-passwd.

Signed-off-by: Jonas Bjurel jonasbjurel@hotmail.com

consultron commented 8 years ago

It works just fine, +2 - this is extremely useful!! One minor comment: Maybe should the help text state that it's the change set number (2557 in https://gerrit.opnfv.org/gerrit/#/c/2557/ for example) that should be used for the -c option?

consultron commented 8 years ago

... will look a little more though, I see that I don't get the exact same commit ID in the repo - want to understand why. :)

jonasbjurel commented 8 years ago

I will hang on until you understand, and fix the help text according to the comment.

consultron commented 8 years ago

OK, got it now, and a small fix is needed to actually verify the input of the -c argument - right now it happily accepts anything and gets going so I was for sure not building what I thought!

I'd suggest using "git ls-remote" to verify the argument and throw an error if not found - or simply check the git return value when cloning the changeset in question?

jonasbjurel commented 8 years ago

So you tried to pull a non existing patch set, and it failed back to something else (master HEAD). Did I get it? I'll fix!

consultron commented 8 years ago

Yes, that's how it seems! I'd be happy to help out as well tomorrow, just say the word!

jonasbjurel commented 8 years ago

I'll try to fix, but sure could need a helping hand;-)

consultron commented 8 years ago

Hi! I've whipped up a standalone example of how I suggest it would work. Use "-c" to checkout a commit id, and "-b" to checkout a branch or a change. If the branch argument is ambiguous, all valid alternatives are listed. checkout.sh.txt

consultron commented 8 years ago

As we agreed I've incorporated this change into this pull request!

jonasbjurel commented 8 years ago

+2 Let's move:-)

jonasbjurel commented 8 years ago

Already +2

BR/Jonas

From: Stefan Berg [mailto:notifications@github.com] Sent: Friday, October 16, 2015 11:11 AM To: jonasbjurel/OPNFV-Playground OPNFV-Playground@noreply.github.com Cc: jonasbjurel jonasbjurel@hotmail.com Subject: Re: [OPNFV-Playground] DO NOT MERGE (#60)

As we agreed I've incorporated this change into this pull request!

— Reply to this email directly or view it on GitHub https://github.com/jonasbjurel/OPNFV-Playground/pull/60#issuecomment-148662167 .