Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 2011/11/07 00:15:48, Graham Percival wrote: On Sun, Nov 06, 2011 at 03:52:15PM +, mailto:adam.spi...@gmail.com wrote: I've done a corresponding patch for changes.tely but I don't have permissions to upload it to this issue that's because Janek uploaded the original issue. Could you just make a new one? just ignore the old codereview issue. Uploading a new patch with git-cl, and associate it with issue tracker 1503 or 1572. - Graham Whoops, I forgot that Carl already created a separate issue for the new version of this patch series: http://codereview.appspot.com/5320074 So this one (4981052) should be ignored entirely now, and closed/deleted when Janek gets back. http://codereview.appspot.com/4981052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
I've done a corresponding patch for changes.tely but I don't have permissions to upload it to this issue so you'll have to get it from here: https://github.com/aspiers/lilypond/commit/3f02826bf5e8854f7da06b2f822d3a65cf63e487 http://codereview.appspot.com/4981052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Sun, Nov 06, 2011 at 03:52:15PM +, adam.spi...@gmail.com wrote: I've done a corresponding patch for changes.tely but I don't have permissions to upload it to this issue that's because Janek uploaded the original issue. Could you just make a new one? just ignore the old codereview issue. Uploading a new patch with git-cl, and associate it with issue tracker 1503 or 1572. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 11-11-06 05:15 PM, Graham Percival wrote: On Sun, Nov 06, 2011 at 03:52:15PM +, adam.spi...@gmail.com wrote: I've done a corresponding patch for changes.tely but I don't have permissions to upload it to this issue that's because Janek uploaded the original issue. Could you just make a new one? just ignore the old codereview issue. Uploading a new patch with git-cl, and associate it with issue tracker 1503 or 1572. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel Would you also close the original Rietveld (code review) issue, please, just in case I ever find the time to go searching through Rietveld for outstanding patches? Thanks, Adam Colin -- I've learned that you shouldn't go through life with a catcher's mitt on both hands. You need to be able to throw something back. -Maya Angelou, poet (1928- ) ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Sun, Nov 06, 2011 at 05:24:25PM -0700, Colin Campbell wrote: Would you also close the original Rietveld (code review) issue, please, just in case I ever find the time to go searching through Rietveld for outstanding patches? can't. Talk to Janek, who's dropped off the internet due to moving apartments. :( - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 11-11-06 05:31 PM, Graham Percival wrote: On Sun, Nov 06, 2011 at 05:24:25PM -0700, Colin Campbell wrote: Would you also close the original Rietveld (code review) issue, please, just in case I ever find the time to go searching through Rietveld for outstanding patches? can't. Talk to Janek, who's dropped off the internet due to moving apartments. :( - Graham Pity, that. (Makes mental note of one more nuisance to remember next time we consider our issue/patch handling system) Colin -- I've learned that you shouldn't go through life with a catcher's mitt on both hands. You need to be able to throw something back. -Maya Angelou, poet (1928- ) ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Thu, Nov 03, 2011 at 06:28:55PM +, Carl Sorensen wrote: On 11/3/11 10:50 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: I originally avoided any pure-whitespace commits, but at the review of my initial patches, I was told to replace tabs with spaces in the files I was modifying: http://codereview.appspot.com/4981052/ I can't find the suggestion to replace tabs with spaces in this review string, so I can't comment on the suggestions. http://codereview.appspot.com/4981052/#msg3 Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. I don't understand why this policy would prevent multiple patches per issue, but you probably shouldn't waste time explaining it to me :) ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 11/4/11 5:35 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: On Thu, Nov 03, 2011 at 06:28:55PM +, Carl Sorensen wrote: On 11/3/11 10:50 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: I originally avoided any pure-whitespace commits, but at the review of my initial patches, I was told to replace tabs with spaces in the files I was modifying: http://codereview.appspot.com/4981052/ I can't find the suggestion to replace tabs with spaces in this review string, so I can't comment on the suggestions. http://codereview.appspot.com/4981052/#msg3 OK, I don't know how I missed that before. Had I seen that review, I'd have mentioned that .scm indentation is not yet a settled issue on the GOP. I'm sorry I didn't catch it earlier. Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. I don't understand why this policy would prevent multiple patches per issue, but you probably shouldn't waste time explaining it to me :) Because the finest granularity we have in the review process is a Rietveld issue. So we only check each Rietveld issue to verify that it doesn't break build. It would be possible to have a series of three commits in a Rietveld issue, with the first two commits breaking the build, and the third fixing the problem. When this set of three is checked on Rietveld, we get a clean make, so the push is approved. If I then push to master as a set of three commits, two of them break build and therefore mess up git-bisect. In the case of your chords patch, I looked over each commit carefully and I'm quite certain that if the build status of the final commit is good, the build status of all previous commits will be good. So I'm comfortable with pushing your chord patches as a set of commits. But in general, I don't think that's a good idea. Thanks, Carl P.S. Your patch is currently up for review, but with a different issue number since I'm now the owner http://codereview.appspot.com/5320074/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
Carl Sorensen c_soren...@byu.edu writes: In the case of your chords patch, I looked over each commit carefully and I'm quite certain that if the build status of the final commit is good, the build status of all previous commits will be good. So I'm comfortable with pushing your chord patches as a set of commits. But in general, I don't think that's a good idea. We are still in the process of figuring out reasonably automatic procedures to admit merge commits (which have the advantage that for the purpose of bisection, the mainline jumps across the series in one piece). They require extra care when rebasing, and extra care is not easy to get at the average git literacy being around. So it is not clear whether it makes sense to try making them a regular part of our toolset. Of course, if most of the work can be automated without all too much pain, this might be worth trying out. The other option is _merging_ instead of fastforwarding dev/staging whenever we are not sure about every single of its components. That makes for a less straightforward history and makes it quite harder, if problems are detected afterwards, to revert the responsible commits without affecting the rest. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Fri, Nov 04, 2011 at 11:55:42AM +, Carl Sorensen wrote: On 11/4/11 5:35 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: On Thu, Nov 03, 2011 at 06:28:55PM +, Carl Sorensen wrote: I can't find the suggestion to replace tabs with spaces in this review string, so I can't comment on the suggestions. http://codereview.appspot.com/4981052/#msg3 OK, I don't know how I missed that before. Had I seen that review, I'd have mentioned that .scm indentation is not yet a settled issue on the GOP. I'm sorry I didn't catch it earlier. No problem :-) Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. I don't understand why this policy would prevent multiple patches per issue, but you probably shouldn't waste time explaining it to me :) Because the finest granularity we have in the review process is a Rietveld issue. So we only check each Rietveld issue to verify that it doesn't break build. It would be possible to have a series of three commits in a Rietveld issue, with the first two commits breaking the build, and the third fixing the problem. When this set of three is checked on Rietveld, we get a clean make, so the push is approved. If I then push to master as a set of three commits, two of them break build and therefore mess up git-bisect. Sure, understood - but I still don't see why each commit within a single issue couldn't be checked accumulatively, rather than just applying all three together and only then doing the check. Of course it's more work, but arguably still less work (and less noise) than creating an issue per commit. In the case of your chords patch, I looked over each commit carefully and I'm quite certain that if the build status of the final commit is good, Sadly there is one regression :-( I've just posted a fix to: http://code.google.com/p/lilypond/issues/detail?id=1503 P.S. Your patch is currently up for review, but with a different issue number since I'm now the owner http://codereview.appspot.com/5320074/ Yep, I got the email alerts for that. Thanks for your work on this! ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 11/4/11 6:10 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: On Fri, Nov 04, 2011 at 11:55:42AM +, Carl Sorensen wrote: On 11/4/11 5:35 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: On Thu, Nov 03, 2011 at 06:28:55PM +, Carl Sorensen wrote: I can't find the suggestion to replace tabs with spaces in this review string, so I can't comment on the suggestions. http://codereview.appspot.com/4981052/#msg3 OK, I don't know how I missed that before. Had I seen that review, I'd have mentioned that .scm indentation is not yet a settled issue on the GOP. I'm sorry I didn't catch it earlier. No problem :-) Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. I don't understand why this policy would prevent multiple patches per issue, but you probably shouldn't waste time explaining it to me :) Because the finest granularity we have in the review process is a Rietveld issue. So we only check each Rietveld issue to verify that it doesn't break build. It would be possible to have a series of three commits in a Rietveld issue, with the first two commits breaking the build, and the third fixing the problem. When this set of three is checked on Rietveld, we get a clean make, so the push is approved. If I then push to master as a set of three commits, two of them break build and therefore mess up git-bisect. Sure, understood - but I still don't see why each commit within a single issue couldn't be checked accumulatively, rather than just applying all three together and only then doing the check. Of course it's more work, but arguably still less work (and less noise) than creating an issue per commit. We could, if we were using something other than Rietveld. Certainly it's less noise than creating an isssue per commit. Our current practice is to create a commit per issue, which is less noise but ends up with bigger commits. I think you're convincing me that we have reason to look at some other review tool in 5 weeks when Graham is ready to face that issue again. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Fri, Nov 04, 2011 at 12:10:20PM +, Adam Spiers wrote: Sure, understood - but I still don't see why each commit within a single issue couldn't be checked accumulatively, rather than just applying all three together and only then doing the check. Because the person checking new patches manually right-clicks on the download raw patch, runs patch -p1 issue1234.diff, manually compiles it, etc etc. correction: after about 4 months of this process, he changed from patch -p1 to using git apply. Bottom line: you're vastly over-estimating our (collective) skill and comfort level with git. It's safe to assume that we're similarly in the dark ages about many other aspects of software development. Over the past 12 months, almost half of our development effort has come from windows users who have never contributed to open source before. They face a pretty steep learning curve. Of course it's more work, but arguably still less work (and less noise) than creating an issue per commit. True, and Patchy could be doing that for us. The brilliant (if I may say so myself) of Patchy is that we don't need to teach everybody how to use moderatly-skilled git commands, we don't need to fumble around manually clicking on website links, etc etc. We don't even need a single person who knows all aspects of Patchy -- as long as people fix little problems with Patchy as we go along, we can end up with a robust automatic system that does whatever we want it to. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Fri, Nov 04, 2011 at 01:14:00PM +, Graham Percival wrote: On Fri, Nov 04, 2011 at 12:10:20PM +, Adam Spiers wrote: Sure, understood - but I still don't see why each commit within a single issue couldn't be checked accumulatively, rather than just applying all three together and only then doing the check. [snipped] Bottom line: you're vastly over-estimating our (collective) skill and comfort level with git. It's safe to assume that we're similarly in the dark ages about many other aspects of software development. Hey, give yourselves some credit - at least you're not using CVS; *that's* what I would label as dark ages :-) git is still relatively new for many many people. Also, being short on experience doesn't make it impossible, it just means it will take a bit longer to get there. Over the past 12 months, almost half of our development effort has come from windows users who have never contributed to open source before. They face a pretty steep learning curve. Sure - I'm finding this a reasonably steep curve, despite the superb documentation etc., and I've been working with F/OSS since 1994. I think that's just the nature of the beast - Lilypond is a large, complex codebase, with many facets to a necessarily complex development process. Sure, it can be improved, but I don't think it's worth beating ourselves up about it either. Of course it's more work, but arguably still less work (and less noise) than creating an issue per commit. True, and Patchy could be doing that for us. The brilliant (if I may say so myself) of Patchy is that we don't need to teach everybody how to use moderatly-skilled git commands, we don't need to fumble around manually clicking on website links, etc etc. We don't even need a single person who knows all aspects of Patchy -- as long as people fix little problems with Patchy as we go along, we can end up with a robust automatic system that does whatever we want it to. I don't know Patchy, but I agree it certainly sounds like a lot of this work could be automated. http://en.wikipedia.org/wiki/Continuous_integration ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Fri, Nov 04, 2011 at 02:52:32PM +, Adam Spiers wrote: Hey, give yourselves some credit - at least you're not using CVS; *that's* what I would label as dark ages :-) git is still relatively new for many many people. Also, being short on experience doesn't make it impossible, it just means it will take a bit longer to get there. Yes and no. Suppose you're a windows users. Suppose you notice a few typos in the lilypond documentation. Suppose you want to be helpful and actually fix it instead of just filing a bug report to bring out attention to the typos. The *shortest / easiest* method we have for those people is: - install virtualbox - download lilydev - use a simplified graphical lily-git.tcl to get our source - edit files in gedit - compile the docs inside the virtual machine - commit and create patch using the simplified graphical lily-git.tcl - send patch by email When most people look at those steps -- which again, is the easiest method we have, after continually looking and revamping this process over the past three years -- they either give up part-way, or just never start. If the task appears to be too hard, it's not a question of time+effort; most people (sensibly) just give up. Sure, it can be improved, but I don't think it's worth beating ourselves up about it either. I'm not beating myself up -- but when I see people spending literally dozens of hours on tasks that could be automated with 1-3 hours of programming, I hardly think that optimism is the appropriate response. :( I don't know Patchy, but I agree it certainly sounds like a lot of this work could be automated. http://en.wikipedia.org/wiki/Continuous_integration Incidently, the latest round of panic is because I switched to ubuntu oneiric 11.10 instead of staying on 10.04 LTS. I wanted the gcc 4.6 toolchain for my Vivi work, but that's caused a huge ripple of disruption in lilypond. Other than setting up a separate computer for lilypond (or maybe dual-booting between ubuntu 10.04 and 11.10), I can't think of how we could have avoided this particular mess. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Fri, Nov 4, 2011 at 3:13 PM, Graham Percival gra...@percival-music.ca wrote: On Fri, Nov 04, 2011 at 02:52:32PM +, Adam Spiers wrote: Hey, give yourselves some credit - at least you're not using CVS; *that's* what I would label as dark ages :-) git is still relatively new for many many people. Also, being short on experience doesn't make it impossible, it just means it will take a bit longer to get there. Yes and no. Suppose you're a windows users. Suppose you notice a few typos in the lilypond documentation. Suppose you want to be helpful and actually fix it instead of just filing a bug report to bring out attention to the typos. The *shortest / easiest* method we have for those people is: - install virtualbox - download lilydev - use a simplified graphical lily-git.tcl to get our source - edit files in gedit - compile the docs inside the virtual machine - commit and create patch using the simplified graphical lily-git.tcl - send patch by email When most people look at those steps -- which again, is the easiest method we have, after continually looking and revamping this process over the past three years -- they either give up part-way, or just never start. If the task appears to be too hard, it's not a question of time+effort; most people (sensibly) just give up. Understood, but I can't really imagine how you could make the initial set up of a build environment for this size of codebase any less of a hurdle. lilydev is a great thing, and even with using it (I didn't), setting up a build environment was a total walk in the park compared to something like digikam. Sure, it can be improved, but I don't think it's worth beating ourselves up about it either. I'm not beating myself up -- but when I see people spending literally dozens of hours on tasks that could be automated with 1-3 hours of programming, I hardly think that optimism is the appropriate response. :( Given that we have this great opportunity to make things way better without too much effort, it seems an entirely appropriate response to me! Surely you wouldn't prefer the automation to be very difficult, or even worse, impossible? I don't know Patchy, but I agree it certainly sounds like a lot of this work could be automated. http://en.wikipedia.org/wiki/Continuous_integration Incidently, the latest round of panic is because I switched to ubuntu oneiric 11.10 instead of staying on 10.04 LTS. I wanted the gcc 4.6 toolchain for my Vivi work, but that's caused a huge ripple of disruption in lilypond. How come? Lilypond compiles fine with gcc 4.6.1 on Fedora 15. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Fri, Nov 04, 2011 at 03:25:30PM +, Adam Spiers wrote: I'm not beating myself up -- but when I see people spending literally dozens of hours on tasks that could be automated with 1-3 hours of programming, I hardly think that optimism is the appropriate response. :( Given that we have this great opportunity to make things way better without too much effort, it seems an entirely appropriate response to me! Surely you wouldn't prefer the automation to be very difficult, or even worse, impossible? heh. no, I'd prefer it if we had more people working on the automation, or more people handling simple tasks so that advanced developers (*cough* like me, if I may be so bold) could work on the automation instead of handling those simple tasks, etc. Incidently, the latest round of panic is because I switched to ubuntu oneiric 11.10 instead of staying on 10.04 LTS. I wanted the gcc 4.6 toolchain for my Vivi work, but that's caused a huge ripple of disruption in lilypond. How come? Lilypond compiles fine with gcc 4.6.1 on Fedora 15. openssh renamed some function SSLv2_client_method, so curl won't compile. This is probably a 5-minute job to just download a newer version of curl, but nobody's stepped forward yet. (this relies on GUB, which is approximately 20,000 lines of python which set up our binary building environemnt -- cross-compiling for freebsd, darwin, mingw, and linux, all on linux) http://code.google.com/p/lilypond/issues/detail?id=1998 we can't compile+run lilypond on 11.10 because of some weird segfault. We can avoid taht by compiling with -fno-crossjumping -fkeep-inline-function, but apparently enabling the latter option *breaks* some old testing code in flower/include/yaffut.h http://code.google.com/p/lilypond/issues/detail?id=1997 http://code.google.com/p/lilypond/issues/detail?id=1875 You may find the latter two issues worthy of skimming. I'm hoping you do, because there's a non-zero chance that you'll recognize some problem, or at least be curious enough to google around a bit, and you may find a simple solution. Or at least, having a fresh pair of eyes on the problem can't hurt. Of all these problems, the inability to compile (without weird options) on ubuntu 11.10 is the worst, since that basically eliminates the automation efforts I've been working on. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
Adam, On Wed, Nov 2, 2011 at 5:58 PM, adam.spi...@gmail.com wrote: I think I am now finished. The new patch series is available at https://github.com/aspiers/lilypond/commits/jazz IMHO there are now too many patches in the series to combine into a single commit for review. To do so would lose a lot of clarity in the git history. So I am not sure how this review process should proceed. One patch per tracker item? Anyhow, you can fetch the patches via: git remote add aspiers git://github.com/aspiers/lilypond.git git fetch aspiers jazz Sorry to belabor the point, but it is unlikely you are going to get much review if those that understand this stuff (I don't, I just push and pull and test formatted patches) have to get patches from a third place. Even if it is just a Rietveld Issue then that is better than having to git fetch from another repo, we in the Bug Squad can create the trackers for you. The patches also adjust the regression tests and English documentation to be consistent with changes to the code. Translation work is required for other languages. http://codereview.appspot.com/4981052/ So should we now be ignoring the stuff in this Rietveld issue? regards -- -- James ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Thu, Nov 03, 2011 at 11:13:28AM +, Peekay Ex wrote: Adam, On Wed, Nov 2, 2011 at 5:58 PM, adam.spi...@gmail.com wrote: I think I am now finished. The new patch series is available at https://github.com/aspiers/lilypond/commits/jazz IMHO there are now too many patches in the series to combine into a single commit for review. To do so would lose a lot of clarity in the git history. So I am not sure how this review process should proceed. One patch per tracker item? I can do that if noone objects to tracker items for patches as trivial as converting tabs to whitespace? Anyhow, you can fetch the patches via: git remote add aspiers git://github.com/aspiers/lilypond.git git fetch aspiers jazz Sorry to belabor the point, but it is unlikely you are going to get much review if those that understand this stuff (I don't, I just push and pull and test formatted patches) have to get patches from a third place. Hmm, well if everyone (including you) is already familiar with 'git pull' then doing 'git fetch' doesn't seem like a big stretch, but OK I'll assume that there are other good reasons for not operating like this. As long as you're OK with a tracker item for even tiny commits then I can work like that. Even if it is just a Rietveld Issue then that is better than having to git fetch from another repo, we in the Bug Squad can create the trackers for you. If Rietveld doesn't support multiple patches per issue then that sounds like a fundamental flaw to me and perhaps it's time to reconsider moving to Gerrit. I'm guessing that combined with a CI tool such as Jenkins it might be able to solve some of the review / staging / release issues which have been mentioned on this list of the last few days - but I don't know anywhere near enough about LilyPond's development yet for that to be more than a guess. https://gerrit.chromium.org/gerrit/#q,status:open,n,z http://source.android.com/source/life-of-a-patch.html The patches also adjust the regression tests and English documentation to be consistent with changes to the code. Translation work is required for other languages. http://codereview.appspot.com/4981052/ So should we now be ignoring the stuff in this Rietveld issue? Yes, they're old now. In light of the above I guess it will have to be closed and superceded by new issues. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Thu, Nov 03, 2011 at 04:00:41PM +, Adam Spiers wrote: On Thu, Nov 03, 2011 at 11:13:28AM +, Peekay Ex wrote: One patch per tracker item? I can do that if noone objects to tracker items for patches as trivial as converting tabs to whitespace? I'd rather not see those patches. Hmm, I'm seeing 11 patches? how hard would it be to do some intelligent rebasing here? i.e. rebase any programming features / bugfixes into one (or more) patches, rebase the ly file fixes into one (or more) patches, etc. I mean, c6fe8a can easily be... oh wait, no it can't. MAO! we don't like changes like that. We really, really don't like changes like that. Could I interest you in scheme indentation: http://lilypond.org/~graham/gop/gop_10.html with about 30 - 90 minutes of work, we can settle these IDIOTIC indentation commits once and for all. Get the tool finalized, run it on all the scm files, and then celebrate. We (finally) did this with C++ over the summer... the whole debate and work on the tools took at least 40 hours of developer time, but it was worth it. Unfortunately, the scheme indentation stuff stalled in August due to a number of factors. Which was a shame, because scheme indentation is WAY easier than C++ indentation, and also because the indentation script was almost finished. Sorry to belabor the point, but it is unlikely you are going to get much review if those that understand this stuff (I don't, I just push and pull and test formatted patches) have to get patches from a third place. Hmm, well if everyone (including you) is already familiar with 'git pull' then doing 'git fetch' doesn't seem like a big stretch, We're not comfortable with git. Other than 4 or 5 people, each person who's started pushing to dev/staging has required between 3 and 10 emails to get them able to reliably push to a branch without screwing stuff up. If Rietveld doesn't support multiple patches per issue then that sounds like a fundamental flaw to me and perhaps it's time to reconsider moving to Gerrit. Stop right there. This debate has chewed up about 25 hours of developer time so far, with no end in sight. I realize that you're an excellent person to move it forward, but I don't want to hear about it right now. I'm vetoing this discussion for 5 weeks. Wait until you know us better (in particular, the relative lack of technical ability), wait until we know you better, wait until our Grand Organization Project starts up again. this, incidently, is exactly GOP 13: http://lilypond.org/~graham/gop/gop_13.html Moving back to the jazz patches: Carl, could you take a look at his git repo and suggest any way of moving forward? Also, with no disrespect intended, let's leave James out of the loop now. We need a senior developer looking at this, not our only documentation writer. - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Thu, Nov 03, 2011 at 04:28:53PM +, Graham Percival wrote: On Thu, Nov 03, 2011 at 04:00:41PM +, Adam Spiers wrote: On Thu, Nov 03, 2011 at 11:13:28AM +, Peekay Ex wrote: One patch per tracker item? I can do that if noone objects to tracker items for patches as trivial as converting tabs to whitespace? I'd rather not see those patches. Hmm, I'm seeing 11 patches? how hard would it be to do some intelligent rebasing here? i.e. rebase any programming features / bugfixes into one (or more) patches, rebase the ly file fixes into one (or more) patches, etc. I always do a lot of patch sculpting via git rebase -i before I deem my patches to be good enough to push upstream. So what you are seeing have already been rebased many times in order to obtain commits which logically group changes together. For example, each new feature being introduced is accompanied by its corresponding doc tweaks and regression tests where relevant. That said, I *could* squash a few of the smaller patches together. But I still think it would be a mistake to squash all these into one or two commits, which would then lack a lot of clarity. I mean, c6fe8a can easily be... oh wait, no it can't. MAO! we don't like changes like that. We really, really don't like changes like that. Could I interest you in scheme indentation: http://lilypond.org/~graham/gop/gop_10.html with about 30 - 90 minutes of work, we can settle these IDIOTIC indentation commits once and for all. Get the tool finalized, run it on all the scm files, and then celebrate. We (finally) did this with C++ over the summer... the whole debate and work on the tools took at least 40 hours of developer time, but it was worth it. Unfortunately, the scheme indentation stuff stalled in August due to a number of factors. Which was a shame, because scheme indentation is WAY easier than C++ indentation, and also because the indentation script was almost finished. Understood. I've already spent WAY more time on all this than budgeted, so I'm not sure I can afford to stretch any further for a while. I originally avoided any pure-whitespace commits, but at the review of my initial patches, I was told to replace tabs with spaces in the files I was modifying: http://codereview.appspot.com/4981052/ and given my strong aversion to (a) a mix of indentation styles within a single file, and (b) commits which mix whitespace changes with real coding changes, I thought this was the best way to proceed. But I agree the sensible thing would be to fix all .scm files in one go. Sorry to belabor the point, but it is unlikely you are going to get much review if those that understand this stuff (I don't, I just push and pull and test formatted patches) have to get patches from a third place. Hmm, well if everyone (including you) is already familiar with 'git pull' then doing 'git fetch' doesn't seem like a big stretch, We're not comfortable with git. Other than 4 or 5 people, each person who's started pushing to dev/staging has required between 3 and 10 emails to get them able to reliably push to a branch without screwing stuff up. Ahah, OK. If Rietveld doesn't support multiple patches per issue then that sounds like a fundamental flaw to me and perhaps it's time to reconsider moving to Gerrit. Stop right there. This debate has chewed up about 25 hours of developer time so far, with no end in sight. I realize that you're an excellent person to move it forward, but I don't want to hear about it right now. I'm vetoing this discussion for 5 weeks. Wait until you know us better (in particular, the relative lack of technical ability), wait until we know you better, wait until our Grand Organization Project starts up again. this, incidently, is exactly GOP 13: http://lilypond.org/~graham/gop/gop_13.html OK, I couldn't remember whether I'd already seen it somewhere :-) Just wanted to make sure it's on people's radar, so given GOP 13 I'm happy not to mention it again :-) Moving back to the jazz patches: Carl, could you take a look at his git repo and suggest any way of moving forward? Thanks, guidance would be welcome. ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Thu, Nov 3, 2011 at 4:28 PM, Graham Percival gra...@percival-music.ca wrote: We're not comfortable with git. Other than 4 or 5 people, each person who's started pushing to dev/staging has required between 3 and 10 emails to get them able to reliably push to a branch without screwing stuff up. I just discovered this git tutorial site which may be of use to some: http://think-like-a-git.net/sections/who-this-site-is-for.html I've seen many others, but this one looks really great, and is pretty funny too (to my twisted sense of humour, at least). ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 11/3/11 10:50 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: On Thu, Nov 03, 2011 at 04:28:53PM +, Graham Percival wrote: On Thu, Nov 03, 2011 at 04:00:41PM +, Adam Spiers wrote: On Thu, Nov 03, 2011 at 11:13:28AM +, Peekay Ex wrote: One patch per tracker item? I can do that if noone objects to tracker items for patches as trivial as converting tabs to whitespace? I'd rather not see those patches. Hmm, I'm seeing 11 patches? how hard would it be to do some intelligent rebasing here? i.e. rebase any programming features / bugfixes into one (or more) patches, rebase the ly file fixes into one (or more) patches, etc. I always do a lot of patch sculpting via git rebase -i before I deem my patches to be good enough to push upstream. So what you are seeing have already been rebased many times in order to obtain commits which logically group changes together. For example, each new feature being introduced is accompanied by its corresponding doc tweaks and regression tests where relevant. That said, I *could* squash a few of the smaller patches together. But I still think it would be a mistake to squash all these into one or two commits, which would then lack a lot of clarity. As long as each commit can successfully build the regtests, it's good. My main concern about the series of commits as one issue is that there is no check to prove that we can build docs and regtests. Unfortunately, the scheme indentation stuff stalled in August due to a number of factors. Which was a shame, because scheme indentation is WAY easier than C++ indentation, and also because the indentation script was almost finished. Understood. I've already spent WAY more time on all this than budgeted, so I'm not sure I can afford to stretch any further for a while. I originally avoided any pure-whitespace commits, but at the review of my initial patches, I was told to replace tabs with spaces in the files I was modifying: http://codereview.appspot.com/4981052/ I can't find the suggestion to replace tabs with spaces in this review string, so I can't comment on the suggestions. and given my strong aversion to (a) a mix of indentation styles within a single file, and (b) commits which mix whitespace changes with real coding changes, I thought this was the best way to proceed. But I agree the sensible thing would be to fix all .scm files in one go. There were actually some comments from senior developers opposing this plan, which was part of the reason it's never been finished. If Rietveld doesn't support multiple patches per issue then that sounds like a fundamental flaw to me and perhaps it's time to reconsider moving to Gerrit. Stop right there. This debate has chewed up about 25 hours of developer time so far, with no end in sight. I realize that you're an excellent person to move it forward, but I don't want to hear about it right now. I'm vetoing this discussion for 5 weeks. Wait until you know us better (in particular, the relative lack of technical ability), wait until we know you better, wait until our Grand Organization Project starts up again. this, incidently, is exactly GOP 13: http://lilypond.org/~graham/gop/gop_13.html OK, I couldn't remember whether I'd already seen it somewhere :-) Just wanted to make sure it's on people's radar, so given GOP 13 I'm happy not to mention it again :-) Rietveld doesn't prevent multiple patches per issue. Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. Moving back to the jazz patches: Carl, could you take a look at his git repo and suggest any way of moving forward? Thanks, guidance would be welcome. I will do so, but probably not until Saturday. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On Thu, Nov 03, 2011 at 06:28:55PM +, Carl Sorensen wrote: On 11/3/11 10:50 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: and given my strong aversion to (a) a mix of indentation styles within a single file, and (b) commits which mix whitespace changes with real coding changes, I thought this was the best way to proceed. But I agree the sensible thing would be to fix all .scm files in one go. There were actually some comments from senior developers opposing this plan, which was part of the reason it's never been finished. My understanding is that Jan insisted on the script matching emacs' formatting, but as long as that's done he won't object. (come to think of it, that may have been private email after the main discussion died down) It's a shame that emacs doesn't support the official extension language of GNU, otherwise we could just use their .elisp indentation directly. :( Rietveld doesn't prevent multiple patches per issue. Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. I thought that rietveld automatically squashed all patches into a single large diff -- which I agree isn't precisely the same as preventing multiple patches, but it doesn't precisely support them either. :) - Graham ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
Graham Percival gra...@percival-music.ca writes: On Thu, Nov 03, 2011 at 06:28:55PM +, Carl Sorensen wrote: On 11/3/11 10:50 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: and given my strong aversion to (a) a mix of indentation styles within a single file, and (b) commits which mix whitespace changes with real coding changes, I thought this was the best way to proceed. But I agree the sensible thing would be to fix all .scm files in one go. There were actually some comments from senior developers opposing this plan, which was part of the reason it's never been finished. My understanding is that Jan insisted on the script matching emacs' formatting, but as long as that's done he won't object. (come to think of it, that may have been private email after the main discussion died down) It's a shame that emacs doesn't support the official extension language of GNU, otherwise we could just use their .elisp indentation directly. :( Hm? Emacs uses scheme-mode for .scm files. Where is the problem with that? Rietveld doesn't prevent multiple patches per issue. Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. I thought that rietveld automatically squashed all patches into a single large diff -- which I agree isn't precisely the same as preventing multiple patches, but it doesn't precisely support them either. :) You can just upload them as a sequence. But that's really not the same as different revisions. -- David Kastrup ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 11/3/11 1:13 PM, David Kastrup d...@gnu.org wrote: Graham Percival gra...@percival-music.ca writes: My understanding is that Jan insisted on the script matching emacs' formatting, but as long as that's done he won't object. (come to think of it, that may have been private email after the main discussion died down) It's a shame that emacs doesn't support the official extension language of GNU, otherwise we could just use their .elisp indentation directly. :( Hm? Emacs uses scheme-mode for .scm files. Where is the problem with that? .elisp files aren't valid Guile files, so we can't run them with Guile. One of our goals is to avoid requiring people to use emacs -- for those who don't use it as their editor, it seems silly to download and install emacs just to get indentation. Rietveld doesn't prevent multiple patches per issue. Our policy of testing each commit to ensure that it doesn't break build prevents multiple patches per issue. I thought that rietveld automatically squashed all patches into a single large diff -- which I agree isn't precisely the same as preventing multiple patches, but it doesn't precisely support them either. :) You can just upload them as a sequence. But that's really not the same as different revisions. We don't have to (and IMO shouldn't) use downloaded patches from Rietveld for pushing, because they lose all author information. But we should insist on one commit per Rietveld issue (or per reviewed chunk), unless we have some other method of verifying that the build is not broken by intermediate commits. Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
On 11/3/11 10:50 AM, Adam Spiers lilypond-de...@adamspiers.org wrote: On Thu, Nov 03, 2011 at 04:28:53PM +, Graham Percival wrote: On Thu, Nov 03, 2011 at 04:00:41PM +, Adam Spiers wrote: On Thu, Nov 03, 2011 at 11:13:28AM +, Peekay Ex wrote: Moving back to the jazz patches: Carl, could you take a look at his git repo and suggest any way of moving forward? Thanks, guidance would be welcome. I've pushed his patches for review. I can see nothing troublesome about his commits and the way he has separated things. I believe these patches are exceptionally clean. If they pass regtests (which I *haven't* checked), I'm glad to have them committed as-is (and I will do so). Thanks, Carl ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
I think I am now finished. The new patch series is available at https://github.com/aspiers/lilypond/commits/jazz IMHO there are now too many patches in the series to combine into a single commit for review. To do so would lose a lot of clarity in the git history. So I am not sure how this review process should proceed. Anyhow, you can fetch the patches via: git remote add aspiers git://github.com/aspiers/lilypond.git git fetch aspiers jazz The patches also adjust the regression tests and English documentation to be consistent with changes to the code. Translation work is required for other languages. http://codereview.appspot.com/4981052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
I have finally figured out how to reproduce these regressions, and am working on a fix. http://codereview.appspot.com/4981052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
Passes make test but fails reg test check. See: http://code.google.com/p/lilypond/issues/detail?id=1572#c7 for more information James http://codereview.appspot.com/4981052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
changes in chord names formatting (1503, 1572) (issue 4981052)
Reviewers: adam.spiers,
Message:
Patches from Adam Spiers.
Description:
http://code.google.com/p/lilypond/issues/detail?id=1503
http://code.google.com/p/lilypond/issues/detail?id=1572
1503 - Allow choice of prefix for chord modifiers.
This was previously add, e.g. Cmaj7 add6add9,
but this results in too much clutter and
is rarely used.
Issue 1503 - Allow choice of minor chord modifier.
For example, often it is preferred to use a hyphen
instead of m. This can now be achieved via:
\set minorChordModifier = \markup { - }
add chordInversionSeparator
Issue 1572 and issue 1503 - Allow choice of
chord modifier separator independently of
chord inversion separator, since conventionally
the latter is always a slash (hence the term
slash chords), whereas the former seldom
involves slashes.
Issue 1503 - Recognise Lydian chords
enlarge half-diminished slashed circle symbol
Please review this at http://codereview.appspot.com/4981052/
Affected files:
M ly/chord-modifiers-init.ly
M ly/engraver-init.ly
M scm/chord-ignatzek-names.scm
M scm/define-context-properties.scm
Index: ly/chord-modifiers-init.ly
diff --git a/ly/chord-modifiers-init.ly b/ly/chord-modifiers-init.ly
index
75b804bb051aeb455676df081c5960d03485997a..91cb1e1fc15bd989fc5ed85daf5d68554c4eb20d
100644
--- a/ly/chord-modifiers-init.ly
+++ b/ly/chord-modifiers-init.ly
@@ -27,9 +27,10 @@ ignatzekExceptionMusic = {
c es ges-\markup { \super o } % should be $\circ$ ?
c es ges bes-\markup {
%% f8 is o with slash.
- \super #(ly:export (ly:wide-char-utf-8 #x00f8))
+ \normal-size-super #(ly:export (ly:wide-char-utf-8 #x00f8))
}
c es ges beses-\markup { \super o7 }
+ c e g b fis'-\markup { \super lyd } % Lydian
}
partialJazzMusic = {
Index: ly/engraver-init.ly
diff --git a/ly/engraver-init.ly b/ly/engraver-init.ly
index
a2ba6551f678d11061d13afae9d6bcc90a6946a1..37cd695e383e1147e010123990f1ebb93f0160b4
100644
--- a/ly/engraver-init.ly
+++ b/ly/engraver-init.ly
@@ -641,9 +641,12 @@ automatically when an output definition (a
@code{\score} or
%% chord names:
chordNameFunction = #ignatzek-chord-names
+ minorChordModifier = m
+ additionalPitchPrefix = % was add
majorSevenSymbol = #whiteTriangleMarkup
chordNameLowercaseMinor = ##f
- chordNameSeparator = #(make-simple-markup /)
+ chordNameSeparator = #(make-hspace-markup 0.5)
+ chordInversionSeparator = #(make-simple-markup /)
chordNameExceptions = #ignatzekExceptions
chordNoteNamer = #'()
chordRootNamer = #note-name-markup
Index: scm/chord-ignatzek-names.scm
diff --git a/scm/chord-ignatzek-names.scm b/scm/chord-ignatzek-names.scm
index
696d02fc7af43ab3ae2383f670a51ecc98afde9d..0302f35e82f7ab8472ac1f222f5d3b0515bf3597
100644
--- a/scm/chord-ignatzek-names.scm
+++ b/scm/chord-ignatzek-names.scm
@@ -130,7 +130,8 @@ work than classifying the pitches.
(define (prefix-modifier-markup mod)
(if (and (= 3 (pitch-step mod))
(= FLAT (ly:pitch-alteration mod)))
- (make-simple-markup (if lowercase-root? m))
+ (if lowercase-root? (empty-markup)
+ (ly:context-property context 'minorChordModifier))
(make-simple-markup huh)))
(define (filter-alterations alters)
@@ -168,8 +169,10 @@ work than classifying the pitches.
(make-line-markup total)))
(let* ((sep (ly:context-property context 'chordNameSeparator))
+ (invsep (ly:context-property context 'chordInversionSeparator))
(root-markup (name-root root lowercase-root?))
- (add-markups (map (lambda (x) (glue-word-to-step add x))
+ (add-pitch-prefix (ly:context-property
context 'additionalPitchPrefix))
+ (add-markups (map (lambda (x) (glue-word-to-step add-pitch-prefix x))
addition-pitches))
(filtered-alterations (filter-alterations alteration-pitches))
(alterations (map name-step filtered-alterations))
@@ -183,7 +186,7 @@ work than classifying the pitches.
suffixes
add-markups) sep))
(base-stuff (if (ly:pitch? bass-pitch)
- (list sep (name-note bass-pitch #f))
+ (list invsep (name-note bass-pitch #f))
'(
(set! base-stuff
Index: scm/define-context-properties.scm
diff --git a/scm/define-context-properties.scm
b/scm/define-context-properties.scm
index
e9acd9690713e8924844cda7d0ff90f3f565a16c..e03cd1076b209d8496424cc645e4fbd13b47b432
100644
--- a/scm/define-context-properties.scm
+++ b/scm/define-context-properties.scm
@@ -45,6 +45,8 @@
;; TODO FIXME
(aDueText ,markup? Text to print at a unisono passage.)
+ (additionalPitchPrefix ,string? Text with which to prefix
+additional pitches within a chord name.)
(alignAboveContext ,string? Where to insert newly created context in
vertical alignment.)
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
LGTM http://codereview.appspot.com/4981052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
Re: changes in chord names formatting (1503, 1572) (issue 4981052)
LGTM AFAICT, but I am not familiar with chord notation. Presumably there should be some doc changes? These can either be included with this patch or separately. If the latter, please make a new issue for the docs when this is pushed so we don't forget. http://codereview.appspot.com/4981052/diff/1/ly/chord-modifiers-init.ly File ly/chord-modifiers-init.ly (right): http://codereview.appspot.com/4981052/diff/1/ly/chord-modifiers-init.ly#newcode30 ly/chord-modifiers-init.ly:30: \normal-size-super #(ly:export (ly:wide-char-utf-8 #x00f8)) please replace tab with spaces (I know you didn't insert it, but we want to remove tabs in the source ASAP) http://codereview.appspot.com/4981052/diff/1/scm/chord-ignatzek-names.scm File scm/chord-ignatzek-names.scm (right): http://codereview.appspot.com/4981052/diff/1/scm/chord-ignatzek-names.scm#newcode175 scm/chord-ignatzek-names.scm:175: (add-markups (map (lambda (x) (glue-word-to-step add-pitch-prefix x)) tab http://codereview.appspot.com/4981052/diff/1/scm/chord-ignatzek-names.scm#newcode189 scm/chord-ignatzek-names.scm:189: (list invsep (name-note bass-pitch #f)) tabs http://codereview.appspot.com/4981052/ ___ lilypond-devel mailing list lilypond-devel@gnu.org https://lists.gnu.org/mailman/listinfo/lilypond-devel
