Nice review. I will make changes in response to your comments.
Thanks, Carl 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 {} { On 2012/01/21 21:12:58, janek wrote:
this isn't used anywhere, we keep it just in case?
Looks like it can go. Thanks for the catch. http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode252 scripts/auxiliar/lily-git.tcl:252: if {$workingBranch != ""} { On 2012/01/21 21:12:58, janek wrote:
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)?
No. workingBranch should never be null. If it is, we'll get lots of git errors. So if workingBranch is null, we don't do any checking out. No. $workingBranch should never be null. If you try to ch http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode253 scripts/auxiliar/lily-git.tcl:253: add_working_branch On 2012/01/21 21:12:58, janek wrote:
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?
Precisely. http://codereview.appspot.com/5504092/diff/14001/scripts/auxiliar/lily-git.tcl#newcode270 scripts/auxiliar/lily-git.tcl:270: git rebase origin/$originHead On 2012/01/21 21:12:58, janek wrote:
shouldn't we update local master too? If we don't do this, how can we not run into trouble in line 309?
Ahh. I see what you mean. Before we were only rebasing master to origin/master. Then we changed to rebasing workingBranch to origin/master. We should be rebasing master to origin/master, and then workingBranch to master. Thanks for the catch. 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)}] { On 2012/01/21 21:12:58, janek wrote:
does this mean "it there's no external variable to control
workingBranch"? Yes. Strictly speaking it means "If there is an error in setting the value of the local variable workingBranch to the environment variable LILYPOND_BRANCH". http://codereview.appspot.com/5504092/ _______________________________________________ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
