Re: Update lilygit.tcl (Issue 2092) (issue 5504092)

[janek . lilypond]

Some questions and concerns.

thanks,
Janek


http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl
File scripts/auxiliar/lily-git.tcl (right):

http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode222
scripts/auxiliar/lily-git.tcl:222: proc update_lilypond_norebase {} {
this isn't used anywhere, we keep it just in case?

http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode252
scripts/auxiliar/lily-git.tcl:252: if {$workingBranch != ""} {
Do i understand correctly that we check workingBranch and switch to
working on it if it's not empty?  So, if we don't want to work on
workingBranch, we set it to null and lily-git will remain on OriginHead
(i.e. master)?

http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode253
scripts/auxiliar/lily-git.tcl:253: add_working_branch
Won't this reset our workingBranch to be a copy of master every time we
update repository?
Wait, i see: it's done only when creating new repository?

http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode270
scripts/auxiliar/lily-git.tcl:270: git rebase origin/$originHead
shouldn't we update local master too?
If we don't do this, how can we not run into trouble in line 309?

http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode272
scripts/auxiliar/lily-git.tcl:272: git merge origin/$originHead
Shouldn't we checkout workingBranch first (after checking if it exists)?

http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode309
scripts/auxiliar/lily-git.tcl:309: git rebase $originHead
isn't there a risk that master is less up-to-date than working-branch?
(see comment on line 270)

http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode560
scripts/auxiliar/lily-git.tcl:560: if [catch {set workingBranch
$env(LILYPOND_BRANCH)}] {
does this mean "it there's no external variable to control
workingBranch"?

http://codereview.appspot.com/5504092/