Re: [Audacity-devel] Patch: unused defines & code cleanup
A free multi-track audio editor and recorder
Brought to you by:
aosiniao
From: Vaughan J. <va...@au...> - 2013-09-25 02:17:41
|
On 9/24/2013 5:46 PM, Martyn Shaw wrote: > > > On 24/09/2013 06:38, Vaughan Johnson wrote: >> On 9/23/2013 4:43 PM, Martyn Shaw wrote: >>> >>> >>> On 23/09/2013 23:49, Leland wrote: >>>> On 9/23/2013 4:45 PM, Vaughan Johnson wrote: >>>>> >>>>> On 9/23/2013 1:16 PM, Richard Ash wrote: >>>>>> On Sun, 22 Sep 2013 12:31:25 +1000 >>>>>> Campbell Barton <ide...@gm...> wrote: >>>>>> >>>>>>> This patch removes unused defines and comments some unused blocks of >>>>>>> code (maybe whoever checks the patch would prefer to remove this >>>>>>> code >>>>>>> instead). >>>>>>> >>>>>>> http://www.graphicall.org/ftp/ideasman42/audacity_unused_defines_and_code_r12582.diff >>>>>>> >>>>>>> >>>>>> >>>>>> I have committed all of this (converting #if 0 to deletion), >>>>>> except the >>>>>> unused SortCallback() function in prefs/KeyConfigPrefs.cpp. >>>>> >>>>> I know that Leland's habit is to use #if 0 instead of commenting out >>>>> code, so I'll ask him to check the commit, to see whether there's >>>>> actually anything he wants left in there. >>>>> >>>> It was related to this thread: >>>> >>>> http://sourceforge.net/p/audacity/mailman/message/22600761/ >> >> Regardless of that particular commit, "#if 0" is not very standard >> practice, afaik. And that's why Barton thought they were cruft. > > Actually, I reviewed all of those commits made by Richard yesterday, and > suggested by Campbell, since I was worried about them as well. Thanks, Martyn! >I think > that Campbell is using some very powerful tools to automatically find > what is 'cruft' and what is not. And Richard is using oversight to > decide what to commit and what to not. I don't see any removal of code > that might be useful, and a great tidy-up going on! Good work guys! +1. - V > > I do agree that commented-out code should be commented on though. > > TTFN > Martyn > >> I meant to write "commits", not just "commit". I haven't reviewed >> Richard's commits of his amended/split version of Campbell's patch in >> detail, but I think there were probably several "#if 0" sections removed >> in those. >> >> I am concerned that maybe too much has been removed with Richard's >> commits, because of "#if 0" being assumed to be cruft, that Leland did >> not mean to leave as cruft, but as work in progress. >> >> Leland, if you have time, please review the others. Thanks! >> >> >>>> >>>> Looks like I went to bed and never finished the sorting. Basically, it >>>> was supposed to sort the list when the column headings were clicked. >>>> >>>> So, I can either finish that now or deleted the unused code. >> >> Your choice, Leland, but if you do not finish it, please comment the >> code as to what's going on -- why it's a partial solution, what the goal >> is -- basically why it's still there. >> >> I think commented-out code is fine, if it's better if clearly stated how >> it's on the path to improvement. >> >> >>>> >>>> What say ye? >>> >>> I say, continue your useful work with the other stuff, and come back >>> to this in the future. >> >> Yes, but please put in a comment about how it's tentative and in >> progress, regardless whether it's commented out per C/C++ comments or >> '#if 0'. We have a wide range of styles on this project. And sometimes >> sporadic but very welcome contributions -- so commenting about work in >> progress is especially helpful. >> >> If it's commented out via "//" or <"/*"..."*/"> or "#if 0", it should be >> explained for people trying to figure it out in the future. >> >> >> Thanks, all!!! >> >> - V >> >> ------------------------------------------------------------------------------ >> >> October Webinars: Code for Performance >> Free Intel webinars can help you accelerate application performance. >> Explore tips for MPI, OpenMP, advanced profiling, and more. Get the >> most from >> the latest Intel processors and coprocessors. See abstracts and >> register > >> http://pubads.g.doubleclick.net/gampad/clk?id=60133471&iu=/4140/ostg.clktrk >> >> _______________________________________________ >> audacity-devel mailing list >> aud...@li... >> https://lists.sourceforge.net/lists/listinfo/audacity-devel >> > |