Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
Is it nestable? (Can you put one of these spanners inside of another?) --- Christopher Heckman https://codereview.appspot.com/571180043/
Re: Issue 5605: implement original () with covariant return types (issue 577110043 by nine.fierce.ball...@gmail.com)
LGTM https://codereview.appspot.com/577110043/
Re: What is holding up 2.20 release?
Carl Sorensen writes: > On 11/16/19, 2:14 PM, "David Kastrup" wrote: > > Carl Sorensen writes: > > > On 11/16/19, 1:52 PM, "David Kastrup" wrote: > > > > Carl Sorensen writes: > > > > > Dear Team, > > > > > > It seems to me like we are pretty much in shape such that we > should > > > release 2.20. I'd be fine if we called 2.19.83-1 the 2.20 > release, > > > even if there are some critical regressions. 2.19.83 is SO much > > > better than 2.18.2. > > > > > > IIUC, the only thing 2.20 is waiting on is for David K. to > cherry-pick > > > some patches. Is that correct? > > > > And putting out a new prerelease to be sure that those are ok, and > > waiting for the translators to catch up with cherry-picked patches > > containing stuff to be translated. > > > > But the current roadblock is David K. cherry-picking some patches. > > > > Is the reason for cherry-picking such a big list of patches to avoid > > some regressions? > > THIS IS NOT A LIST FOR CHERRY-PICKING It is a unculled list of > potential candidates that _may_ need or want to be cherry-picked, in > case a particular candidate is > > either > a) fixing a regression > b) fixing a problem that will foreseeably cause trouble with near-future > or our current build systems > c) fixing a problem or providing a feature that will foreseeably cause > frequent tension between 2.20 and 2.21 users if not cherry-picked > d) a definite improvement that does not show potential for causing new > regressions > e) a documentation fix/change matching 2.20 behavior > > and/or/maybe > a) cherry-picks reasonably painlessly > b) does not cause significant followup tasks to be also scheduled > > > Or are these patches that were created after we put out the last > > pre-release? > > Minus those I already cherry-picked > > > Would you like me to try doing the cherry-picking for you? > > If anybody tries indiscriminately picking that list, I am going to be > pissed. The majority of those patches likely has no place in 2.20. > > OK, I will not do any cherry-picking. > > Is there anything else that can be done to help? I already mentioned _vetting_ that list but you removed that part before replying. -- David Kastrup
Re: What is holding up 2.20 release?
On 11/16/19, 2:14 PM, "David Kastrup" wrote: Carl Sorensen writes: > On 11/16/19, 1:52 PM, "David Kastrup" wrote: > > Carl Sorensen writes: > > > Dear Team, > > > > It seems to me like we are pretty much in shape such that we should > > release 2.20. I'd be fine if we called 2.19.83-1 the 2.20 release, > > even if there are some critical regressions. 2.19.83 is SO much > > better than 2.18.2. > > > > IIUC, the only thing 2.20 is waiting on is for David K. to cherry-pick > > some patches. Is that correct? > > And putting out a new prerelease to be sure that those are ok, and > waiting for the translators to catch up with cherry-picked patches > containing stuff to be translated. > > But the current roadblock is David K. cherry-picking some patches. > > Is the reason for cherry-picking such a big list of patches to avoid > some regressions? THIS IS NOT A LIST FOR CHERRY-PICKING It is a unculled list of potential candidates that _may_ need or want to be cherry-picked, in case a particular candidate is either a) fixing a regression b) fixing a problem that will foreseeably cause trouble with near-future or our current build systems c) fixing a problem or providing a feature that will foreseeably cause frequent tension between 2.20 and 2.21 users if not cherry-picked d) a definite improvement that does not show potential for causing new regressions e) a documentation fix/change matching 2.20 behavior and/or/maybe a) cherry-picks reasonably painlessly b) does not cause significant followup tasks to be also scheduled > Or are these patches that were created after we put out the last > pre-release? Minus those I already cherry-picked > Would you like me to try doing the cherry-picking for you? If anybody tries indiscriminately picking that list, I am going to be pissed. The majority of those patches likely has no place in 2.20. OK, I will not do any cherry-picking. Is there anything else that can be done to help? Carl
Re: What is holding up 2.20 release?
David Kastrup writes: > Carl Sorensen writes: > >> On 11/16/19, 1:52 PM, "David Kastrup" wrote: >> >> Carl Sorensen writes: >> >> > Dear Team, >> > >> > It seems to me like we are pretty much in shape such that we should >> > release 2.20. I'd be fine if we called 2.19.83-1 the 2.20 release, >> > even if there are some critical regressions. 2.19.83 is SO much >> > better than 2.18.2. >> > >> > IIUC, the only thing 2.20 is waiting on is for David K. to cherry-pick >> > some patches. Is that correct? >> >> And putting out a new prerelease to be sure that those are ok, and >> waiting for the translators to catch up with cherry-picked patches >> containing stuff to be translated. >> >> But the current roadblock is David K. cherry-picking some patches. >> >> Is the reason for cherry-picking such a big list of patches to avoid >> some regressions? > > THIS IS NOT A LIST FOR CHERRY-PICKING It is a unculled list of > potential candidates that _may_ need or want to be cherry-picked, in > case a particular candidate is > > either > a) fixing a regression > b) fixing a problem that will foreseeably cause trouble with near-future > or our current build systems > c) fixing a problem or providing a feature that will foreseeably cause > frequent tension between 2.20 and 2.21 users if not cherry-picked > d) a definite improvement that does not show potential for causing new > regressions > e) a documentation fix/change matching 2.20 behavior > > and/or/maybe > a) cherry-picks reasonably painlessly > b) does not cause significant followup tasks to be also scheduled > >> Or are these patches that were created after we put out the last >> pre-release? > > Minus those I already cherry-picked or already considered unfit for picking, based on seeing patches succeeding them already getting picked. > >> Would you like me to try doing the cherry-picking for you? > > If anybody tries indiscriminately picking that list, I am going to be > pissed. The majority of those patches likely has no place in 2.20. -- David Kastrup
Re: What is holding up 2.20 release?
Carl Sorensen writes: > On 11/16/19, 1:52 PM, "David Kastrup" wrote: > > Carl Sorensen writes: > > > Dear Team, > > > > It seems to me like we are pretty much in shape such that we should > > release 2.20. I'd be fine if we called 2.19.83-1 the 2.20 release, > > even if there are some critical regressions. 2.19.83 is SO much > > better than 2.18.2. > > > > IIUC, the only thing 2.20 is waiting on is for David K. to cherry-pick > > some patches. Is that correct? > > And putting out a new prerelease to be sure that those are ok, and > waiting for the translators to catch up with cherry-picked patches > containing stuff to be translated. > > But the current roadblock is David K. cherry-picking some patches. > > Is the reason for cherry-picking such a big list of patches to avoid > some regressions? THIS IS NOT A LIST FOR CHERRY-PICKING It is a unculled list of potential candidates that _may_ need or want to be cherry-picked, in case a particular candidate is either a) fixing a regression b) fixing a problem that will foreseeably cause trouble with near-future or our current build systems c) fixing a problem or providing a feature that will foreseeably cause frequent tension between 2.20 and 2.21 users if not cherry-picked d) a definite improvement that does not show potential for causing new regressions e) a documentation fix/change matching 2.20 behavior and/or/maybe a) cherry-picks reasonably painlessly b) does not cause significant followup tasks to be also scheduled > Or are these patches that were created after we put out the last > pre-release? Minus those I already cherry-picked > Would you like me to try doing the cherry-picking for you? If anybody tries indiscriminately picking that list, I am going to be pissed. The majority of those patches likely has no place in 2.20. -- David Kastrup
Re: What is holding up 2.20 release?
On 11/16/19, 1:52 PM, "David Kastrup" wrote: Carl Sorensen writes: > Dear Team, > > It seems to me like we are pretty much in shape such that we should > release 2.20. I'd be fine if we called 2.19.83-1 the 2.20 release, > even if there are some critical regressions. 2.19.83 is SO much > better than 2.18.2. > > IIUC, the only thing 2.20 is waiting on is for David K. to cherry-pick > some patches. Is that correct? And putting out a new prerelease to be sure that those are ok, and waiting for the translators to catch up with cherry-picked patches containing stuff to be translated. But the current roadblock is David K. cherry-picking some patches. Is the reason for cherry-picking such a big list of patches to avoid some regressions? Or are these patches that were created after we put out the last pre-release? Would you like me to try doing the cherry-picking for you? Carl
Re: What is holding up 2.20 release?
Carl Sorensen writes: > Dear Team, > > It seems to me like we are pretty much in shape such that we should > release 2.20. I'd be fine if we called 2.19.83-1 the 2.20 release, > even if there are some critical regressions. 2.19.83 is SO much > better than 2.18.2. > > IIUC, the only thing 2.20 is waiting on is for David K. to cherry-pick > some patches. Is that correct? And putting out a new prerelease to be sure that those are ok, and waiting for the translators to catch up with cherry-picked patches containing stuff to be translated. But the current roadblock is David K. cherry-picking some patches. Here is a remaining list (not completely up to date with current master, though not missing much) to check for possible inclusion (assuming I have not overlooked something important pickable in the sequence before). If you see something important here (or something not in current master), put in a word for it. ef2cc5e684 Issue 5471/1: relocate.cc: Add `indent' parameter to `sane_putenv' d6428b77b6 Issue 5471/2: main.cc, relocate.cc: Improve relocation debug messages 19da23977f Issue 5471/3: relocate.cc: Don't use double forward slashes in file names 88980c4f92 Fix #5474: progerror when creating an undefined grob 0f5c046811 Functions to invert chords or drop/rise chord notes 6f1cf6c2b5 Add regtest for chord inversions and voicings ddef2aa7b6 Issue 5472/1: yaffut.hh: Fix compilation warning e37310a8e9 Issue 5472/2: flower/file-name.cc: Fix canonical file name representation 3bd7050543 Issue 5468: yyout2grammar.py: Various fixes 0deb785f64 Update texinfo.tex from Texinfo git master branch cb1963c46a Web: update online editors section. 965a607096 Fix #1367: NoteNames context in any language ed86e71eb0 Use "simple strings" rather than #"hash-prefixed Scheme strings" 3aeb41c27f Command line help: -o option takes either FILE or FOLDER as arg. e94db23e6c C++ cleanup : get rid of some compilation warnings 804faebc79 Issue 5481/1: main.cc, relocate.cc: Minor code clean-up 7200d7365b Issue 5481/2: running.itely: Document relocation e9c082d4d4 Issue 5481/3: flower/file-name.cc: Better handling of `.' and `..' f0c3e7461e Issue 5481/4: relocate.cc: Rewrite relocation algorithm 5eb848b59b Issue 5484: set doubleRepeatType fallback to default b05de1da2a pitches.itely: Fix typos and improve look of accidental name tables. dceebb052f Website: use VERSION_DEVEL rather than MAJOR.MINOR 5eb392ddd5 Issue 5485: avoid -Wsequence-point warning cdfb8f3fd3 Do not build PDFs from the website Texinfo sources - issue 5482 3ba581df6a Issue 5490: fix for wrong clefs.varC_change 2afb45bd85 Issue 5489/1: improve prall glyphs 56942ccc5f Issue 5489/2: rename trilelement → trillelement 1284c743b0 Issue 5487/1: add very short and Henze fermatas 77b98cbbc6 Issue 5487/2: add new fermatas to Changes doc 5a88042cc8 Scripts: Removed references to gmane 879f5b1da5 Doc: NR 4.2.2 - remove deprecated @knownissue a99f7f83c8 Doc: Usage - Invoking Musicxml2ly - add missing switches 19677c83b0 Makelsr.py run for previous commits 464cf0d811 Fix #687: Include MIDI swing script in default distribution 13dc8b9c72 Issue 5498: Revert "Merge branch 'issue4914'" a39c23d554 Release: bump VERSION_DEVEL. 941dfa8bed Release: update news. 54de6a24c5 Release: bump Welcome versions. 1b1b06dbc7 NR: Document usage of \change at beginning of staff. 45dedd482d Add warning message for unknown code in fret-diagram-verbose 6508284378 Clean up problems with fret-diagram-terse markups 0e6f55bb17 Change hardcoded pair to cons call for return -- remove ugh comment 64a61d9b5d Doc: extending: Fix after-line-breaking example. c05184b3ff CG: Typo for British and American English locales c415cb98c3 Issue 5502: NR: Add many function index entries e0d597aed3 Typo in \oval docstring 0f047e3514 Fix German quarter tone pitch names. f3d46cd65d Fix commit b05de1da2abe0a6d3e0eac8202ea45ba41ff47df. 9c0e1363ac Issue #5506: Add tremolo stem support to \cueDuring 9d9dc55729 Issue 5501: Avoid failed assertion in stem tremolo code 3008803555 Doc: NR added @knownissue re midi block with polymetric notation db818c1220 ly: Adding turkish-makam.ly f4e30857e1 Doc: Usage - Better description of -dcrop in Usage chapter 1.2 a7ef349f29 Doc: LM + NR - Typos \addlyrics and \lyricmode ed0212b1dc Doc: Internals - clarify stem-length is in half staff space units 03f78e3e7b Doc: NR - 2.10.2 - Arabic Music - improved example c511518a4e makelsr run for previous commit d0730c389d Issue 5510 Add doc-string for allowVoltaHook bd05971b2a Issue 5509 Reflect new syntax in example of define-grob-properties.scm 6ecee87252 Minor NR corrections. 8d7c176b45 Issue 5511/1: add articulation support to MMRs 4424b153c0 Issue 5511/2: deprecate \fermataMarkup 453fa92e43 Issue 5511/3: update regression tests 200658ca20 Issue 5511/4: doc: NR and Changes 59284fa692 Issue 5511/5: doc: Update translated docs e8ad7f0c78 Minor. ab8e7e4d89 Issue 5513: Avoid some crashes for bad "control-points" property ff348fa75a Issue 5486/1: add
What is holding up 2.20 release?
Dear Team, It seems to me like we are pretty much in shape such that we should release 2.20. I'd be fine if we called 2.19.83-1 the 2.20 release, even if there are some critical regressions. 2.19.83 is SO much better than 2.18.2. IIUC, the only thing 2.20 is waiting on is for David K. to cherry-pick some patches. Is that correct? I'd really like to see us get something out as 2.20 by the first of the year. What can I do to help? Thanks, Carl
Re: Issue 5603: create just one tree.gittxt file (issue 559260043 by nine.fierce.ball...@gmail.com)
LGTM. Thanks for working on these annoying details! Carl https://codereview.appspot.com/559260043/
Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
On 11/16/19, 7:49 AM, "Kieren MacMillan" wrote: >> I'd vote for MeasureSpanner. > +1 +1 Kieren. MeasureSpanner works for me. Carl
PATCHES - Countdown for November 16th
Hello, Here is the current patch countdown list. The next countdown will be on November 18th. A quick synopsis of all patches currently in the review process can be found here: http://philholmes.net/lilypond/allura/ Push: 5593 Optionally use "tidy" to check generated HTML - Dan Eble https://sourceforge.net/p/testlilyissues/issues/5593 http://codereview.appspot.com/583160043 Countdown: 5600 Fix hidden member templates in derived classes - Jonas Hahnfeld https://sourceforge.net/p/testlilyissues/issues/5600 http://codereview.appspot.com/559250043 5599 Purge remnants of SCons - Jonas Hahnfeld https://sourceforge.net/p/testlilyissues/issues/5599 http://codereview.appspot.com/559240043 5598 More changes for compatbility with future Python versions - Jonas Hahnfeld https://sourceforge.net/p/testlilyissues/issues/5598 http://codereview.appspot.com/554970043 Review: 5604 fix miscellaneous warnings - Dan Eble https://sourceforge.net/p/testlilyissues/issues/5604 http://codereview.appspot.com/577100046 5603 create just one tree.gittxt file - Dan Eble https://sourceforge.net/p/testlilyissues/issues/5603 http://codereview.appspot.com/559260043 5602 Implement MeasureAttachedSpanner - David Nalesnik https://sourceforge.net/p/testlilyissues/issues/5602 http://codereview.appspot.com/571180043 New: No new patches at this time *** Regards, James
Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
Thanks for working on it !!
Some other nits:
https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc
File lily/measure-attached-spanner.cc (right):
https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode93
lily/measure-attached-spanner.cc:93: : ly_symbol2scm ("staff-bar"));
If I understand correctly (and I may be wrong, because my knowledge
about c++ is more or less zero), then "staff-bar" is a fall-back.
I'd create an entry for 'spacing-pair' in define-grobs.scm, too. Similar
to MeasureCounter, MultiMeasure Rest and PercentRepeat.
https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode141
lily/measure-attached-spanner.cc:141: "break-overshoot "
Probably add a regtest for break-overshoot.
Or extent input/regression/spanner-break-overshoot.ly
https://codereview.appspot.com/571180043/
Re[2]: Working on issue 665, how to proceed?
Hi Jaap I've added de-wolff to Sourceforge with Developer permissions. You may now update and create Issues as well as reading them. Next step is to initiate discussion about your patch under Issue 665 at Sourceforge, and upload a patch as a proposal to Rietveld for a code review. Instructions for doing this are in the Contributor's Guide http://lilypond.org/doc/v2.19/Documentation/contributor/quick-start . I see the patch would be quite large, so a code review will be difficult. I'll leave it to the experienced code developers to take it from here. Thanks for working on this - hope it all goes well! Trevor -- Original Message -- From: lilyp...@de-wolff.org To: "'Trevor'" Sent: 16/11/2019 16:53:36 Subject: RE: Working on issue 665, how to proceed? I already have an sourceforge account: my username is de-wolff Jaap -Original Message- From: Trevor Sent: Saturday, November 16, 2019 3:44 PM To: lilyp...@de-wolff.org; lilypond-devel@gnu.org Subject: Re: Working on issue 665, how to proceed? lilyp...@de-wolff.org wrote 15/11/2019 23:31:33 Subject: Working on issue 665, how to proceed? Hi Jaap You wrote >In the last weeks I did write a start of a lilypond -> musicxml solution. > >As this is an (old) issue, there is need to put this in lilypond, but how? >What level of quality is needed? Who can help me to join the lilypond >developer group? The first step is to obtain a username at sourceforge.net and post it to the - devel list so we can add you as a Developer. Then you'll be able to add your comments and code to Issue 665. Trevor
Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
I haven't reviewed the ly or scm. https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh File lily/include/measure-attached-spanner.hh (right): https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode4 lily/include/measure-attached-spanner.hh:4: Copyright (C) 1997--2015 Jan Nieuwenhuizen When you add a new file, I think you're supposed to use (C) . At least, that's what I was once told. https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode20 lily/include/measure-attached-spanner.hh:20: #ifndef Measure_attached_spanner_HH all caps, please https://codereview.appspot.com/571180043/diff/565230043/lily/include/measure-attached-spanner.hh#newcode24 lily/include/measure-attached-spanner.hh:24: #include "std-vector.hh" I don't see anything in this header that uses a vector. Unless I'm wrong, this should be moved to whichever cc files require it. Same goes for any other unnecessary headers. https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc File lily/measure-attached-spanner.cc (right): https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode45 lily/measure-attached-spanner.cc:45: //Direction dir = get_grob_direction (me); remove https://codereview.appspot.com/571180043/diff/565230043/lily/measure-attached-spanner.cc#newcode53 lily/measure-attached-spanner.cc:53: Spanner *orig_spanner = dynamic_cast (me->original ()); If I understand correctly, me->original () can only ever be either null or another instance of the same type as me; therefore, use static_cast here. Also, if it's logically possible for me->original () to be null in this case, check for it before dereferencing below. https://codereview.appspot.com/571180043/
RE: Working on issue 665, how to proceed?
My sourceforge account name is: de-wolff > -Original Message- > From: Trevor > Sent: Saturday, November 16, 2019 3:44 PM > To: lilyp...@de-wolff.org; lilypond-devel@gnu.org > Subject: Re: Working on issue 665, how to proceed? > > lilyp...@de-wolff.org wrote 15/11/2019 23:31:33 > Subject: Working on issue 665, how to proceed? > > Hi Jaap > > You wrote > > >In the last weeks I did write a start of a lilypond -> musicxml solution. > > > >As this is an (old) issue, there is need to put this in lilypond, but how? > >What level of quality is needed? Who can help me to join the lilypond > >developer group? > The first step is to obtain a username at sourceforge.net and post it to the - > devel list so we can add you as a Developer. Then you'll be able to add your > comments and code to Issue 665. > > Trevor
Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
>> I'd vote for MeasureSpanner. > +1 +1 Kieren.
Re[2]: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
-- Original Message -- From: thomasmorle...@gmail.com To: david.nales...@gmail.com; lemzw...@googlemail.com; carl.d.soren...@gmail.com; nine.fierce.ball...@gmail.com Cc: re...@codereview-hr.appspotmail.com; lilypond-devel@gnu.org Sent: 16/11/2019 14:30:43 Subject: Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com) On 2019/11/16 14:27:25, Dan Eble wrote: On 2019/11/15 19:21:09, Carl wrote: > I think the name should be changed from MeasureAttachedSpanner to > BarAttachedSpanner. Calling it just MeasureSpanner would also address the specific problem you raised. Is it more important for the name to state where it is attached or what it spans? I'd vote for MeasureSpanner. +1 Trevor https://codereview.appspot.com/571180043
Re: Working on issue 665, how to proceed?
lilyp...@de-wolff.org wrote 15/11/2019 23:31:33 Subject: Working on issue 665, how to proceed? Hi Jaap You wrote In the last weeks I did write a start of a lilypond -> musicxml solution. As this is an (old) issue, there is need to put this in lilypond, but how? What level of quality is needed? Who can help me to join the lilypond developer group? The first step is to obtain a username at sourceforge.net and post it to the -devel list so we can add you as a Developer. Then you'll be able to add your comments and code to Issue 665. Trevor
Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
https://codereview.appspot.com/571180043/diff/565230043/ly/spanners-init.ly File ly/spanners-init.ly (right): https://codereview.appspot.com/571180043/diff/565230043/ly/spanners-init.ly#newcode25 ly/spanners-init.ly:25: "View side-by-side diff with in-line comments" is broken for this file. https://codereview.appspot.com/571180043/diff/565230043/scm/define-grobs.scm File scm/define-grobs.scm (right): https://codereview.appspot.com/571180043/diff/565230043/scm/define-grobs.scm#newcode1457 scm/define-grobs.scm:1457: ;(font-size . -2) Mmh, this is commented. Why? Same below. https://codereview.appspot.com/571180043/diff/565230043/scm/define-music-types.scm File scm/define-music-types.scm (right): https://codereview.appspot.com/571180043/diff/565230043/scm/define-music-types.scm#newcode311 scm/define-music-types.scm:311: "View side-by-side diff with in-line comments" broken here as well https://codereview.appspot.com/571180043/diff/565230043/scm/scheme-engravers.scm File scm/scheme-engravers.scm (right): https://codereview.appspot.com/571180043/diff/565230043/scm/scheme-engravers.scm#newcode98 scm/scheme-engravers.scm:98: (define-public (Measure_attached_spanner_engraver context) Not related to the current patch: Meanwhile I've seen several scheme-spanners-engravers for new spanner-grobs (and wrote some for my own work) or to customize existing spanner-grobs. All are more or less the same, ofcourse they need to be so. I'm musing whether it would be possible to create some specialized macro. We already have `make-engraver´, maybe something like `make-spanner-engraver´. Thoughts? https://codereview.appspot.com/571180043/
Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
On 2019/11/16 14:27:25, Dan Eble wrote: On 2019/11/15 19:21:09, Carl wrote: > I think the name should be changed from MeasureAttachedSpanner to > BarAttachedSpanner. Calling it just MeasureSpanner would also address the specific problem you raised. Is it more important for the name to state where it is attached or what it spans? I'd vote for MeasureSpanner. https://codereview.appspot.com/571180043/
Re: Implement MeasureAttachedSpanner (issue 571180043 by david.nales...@gmail.com)
On 2019/11/15 19:21:09, Carl wrote: I think the name should be changed from MeasureAttachedSpanner to BarAttachedSpanner. Calling it just MeasureSpanner would also address the specific problem you raised. Is it more important for the name to state where it is attached or what it spans? https://codereview.appspot.com/571180043/
Re: shepherd a patch?
Thanks, James! On Sat, Nov 16, 2019 at 4:07 AM James wrote: > > David, > > On 15/11/2019 13:31, David Nalesnik wrote: > > On Fri, Nov 15, 2019 at 4:31 AM James Lowe wrote: > >> David et al. > >> > >> On Fri, 15 Nov 2019 07:33:01 +0100, Urs Liska > >> wrote: > >> > >>> Hi David, > >>> > >>> I feel responsible for this because I know where this is coming from ;-) > >>> > >>> You can send me the patch. However, it's a long time since I uploaded > >>> anything, so I'm not sure my set-up still works. But I'll try. > >>> > >>> Best > >>> Urs > >>> > >>> Am 15.11.19 um 04:10 schrieb David Nalesnik: > Hi all, > > I have a patch which implements measure-attached spanners. I am > having a devil of a time getting the patch up for review. I simply > can't get git-cl to upload it. (I suspect this has something to do > with the number of files it touches--my system times out before > everything can be uploaded.) > > Would someone be willing to shepherd the patch for me? > > Attached is a PNG which shows the output of one of the regtests. > > This would _really_ be appreciated. Thanks! > > David > >> What I would suggest (generally to anyone on this list that ever needs a > >> patch shepherding) is to make sure a tracker exists - @David, as I know > >> you have access to create tickets you can at least do this - and then > >> attach your patch to that and set the tracker to 'started' so that at the > >> very least if (for example) Urs forgets, goes on holiday, has some > >> emergency, that I can at least pick it up/detect it on the countdown and > >> put it up for review myself. > >> > >> I do this all the time for 'drive by' patches. > >> > >> James > >> > > Thanks for this explanation. I discovered that my patch had gone to > > Rietveld, only no ticket was created. So the error has to do with the > > issue tracker. Here's hoping that it's possible to do what's left > > manually without too much trouble! > > Status = 'started' > > Patch = 'new' > > Those two things will get it on the radar. > > > James >
Re: Issue 5604: fix miscellaneous warnings (issue 577100046 by nine.fierce.ball...@gmail.com)
LGTM https://codereview.appspot.com/577100046/
Re: shepherd a patch?
David, On 15/11/2019 13:31, David Nalesnik wrote: On Fri, Nov 15, 2019 at 4:31 AM James Lowe wrote: David et al. On Fri, 15 Nov 2019 07:33:01 +0100, Urs Liska wrote: Hi David, I feel responsible for this because I know where this is coming from ;-) You can send me the patch. However, it's a long time since I uploaded anything, so I'm not sure my set-up still works. But I'll try. Best Urs Am 15.11.19 um 04:10 schrieb David Nalesnik: Hi all, I have a patch which implements measure-attached spanners. I am having a devil of a time getting the patch up for review. I simply can't get git-cl to upload it. (I suspect this has something to do with the number of files it touches--my system times out before everything can be uploaded.) Would someone be willing to shepherd the patch for me? Attached is a PNG which shows the output of one of the regtests. This would _really_ be appreciated. Thanks! David What I would suggest (generally to anyone on this list that ever needs a patch shepherding) is to make sure a tracker exists - @David, as I know you have access to create tickets you can at least do this - and then attach your patch to that and set the tracker to 'started' so that at the very least if (for example) Urs forgets, goes on holiday, has some emergency, that I can at least pick it up/detect it on the countdown and put it up for review myself. I do this all the time for 'drive by' patches. James Thanks for this explanation. I discovered that my patch had gone to Rietveld, only no ticket was created. So the error has to do with the issue tracker. Here's hoping that it's possible to do what's left manually without too much trouble! Status = 'started' Patch = 'new' Those two things will get it on the radar. James
