Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-18 Thread nine . fierce . ballads
On 2014/10/11 18:52:57, dak wrote: What I usually do is to have my branches associated with an upstream branch. git branch -b issuexxx origin branch is a thinko for checkout right? I have no idea whether one of the two is definitely involved. But keeping the same branch around is

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-18 Thread dak
On 2014/10/18 16:38:14, Dan Eble wrote: On 2014/10/11 18:52:57, dak wrote: What I usually do is to have my branches associated with an upstream branch. git branch -b issuexxx origin branch is a thinko for checkout right? Uh, yes. Sorry. https://codereview.appspot.com/152370043/

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-11 Thread nine . fierce . ballads
On 2014/10/09 05:13:48, dak wrote: Incidentally, do you use git cl for uploading? Your reviews are remarkable in that they do not allow comparing/viewing the various versions of your uploads. That should not usually happen so it would be interesting to know what causes this situation. I have

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-11 Thread dak
On 2014/10/11 17:53:13, Dan Eble wrote: On 2014/10/09 05:13:48, dak wrote: Incidentally, do you use git cl for uploading? Your reviews are remarkable in that they do not allow comparing/viewing the various versions of your uploads. That should not usually happen so it would be

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-08 Thread dak
Neither the Google issue nor the Rietveld review contain any rationale for that change. It complicates the source without _any_ difference in execution as far as I can see. One side change is to remove copy constructors with an assert(false). Personally, I am annoyed at C++ not allowing to

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-08 Thread nine . fierce . ballads
On 2014/10/08 08:42:55, dak wrote: It complicates the source without _any_ difference in execution as far as I can see. The goal of this change is robustness and maintainability. From my perspective, the new version raises fewer questions. One side change is to remove copy constructors with

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-08 Thread dak
On 2014/10/08 13:17:39, Dan Eble wrote: On 2014/10/08 08:42:55, dak wrote: It complicates the source without _any_ difference in execution as far as I can see. The goal of this change is robustness and maintainability. From my perspective, the new version raises fewer questions. You

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-08 Thread nine . fierce . ballads
On 2014/10/08 13:54:07, dak wrote: Not defined is quite definitely not a statement of intent. Nor is it of purpose. It is of fact. And it is quite confusing since it is immediately adjacent to a declaration. It's more like // declared, do not define! Prevents default copy constructor.

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-08 Thread dak
On 2014/10/08 23:48:16, Dan Eble wrote: On 2014/10/08 13:54:07, dak wrote: Not defined is quite definitely not a statement of intent. Nor is it of purpose. It is of fact. And it is quite confusing since it is immediately adjacent to a declaration. It's more like // declared, do not

Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-07 Thread nine . fierce . ballads
Reviewers: , Description: Define Smob constructors. https://code.google.com/p/lilypond/issues/detail?id=4156 Please review this at https://codereview.appspot.com/152370043/ Affected files (+53, -26 lines): M lily/book.cc M lily/context.cc M lily/context-def.cc M lily/font-metric.cc M

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-07 Thread nine . fierce . ballads
Note that I haven't searched for kinds of derived Smob that are still getting compiler-generated constructors. My approach was to clean up SmobT and fix resulting warnings. The rest shouldn't be any worse off now than they were before. https://codereview.appspot.com/152370043/

Re: Issue 4156: Define Smob constructors. (issue 152370043 by nine.fierce.ball...@gmail.com)

2014-10-07 Thread lemzwerg
LGTM, from visual inspection only :-) https://codereview.appspot.com/152370043/diff/1/lily/include/book.hh File lily/include/book.hh (right): https://codereview.appspot.com/152370043/diff/1/lily/include/book.hh#newcode31 lily/include/book.hh:31: typedef SmobBook base_type; I would insert an