Re: Mass Code Cleanup
On 15.02.2010 Robin Anil wrote: > SGD kmeans++ pegasus seems fine. Isabel can you check with the latest trunk > if the perceptron is alright? Any code I had is already checked in. Any examples I am working on should be easy to adopt. Isabel signature.asc Description: This is a digitally signed message part.
Re: Mass Code Cleanup
On 14.02.2010 Grant Ingersoll wrote: > I don't object to good style. I object to sweeping changes that break a > lot of patches. Maybe not the case here, but it will be in the future and > unless the whole thing is automated as part of committing (as Hadoop > does), the code will always have formatting issues causing this exact same > thing to happen. I kind of like the automatic patch checks that are activated for patches in jira over at Hadoop projects. It does highlight trivial problems with the code submitted w/o the need for a code review by a committer. Does anyone here at Mahout know what is needed for such checks? Isabel signature.asc Description: This is a digitally signed message part.
Re: Mass Code Cleanup
I picked out the formatter issues and committed the rest. Will have smaller patches if anything looks horribly machine formatter. So far not much Robin On Mon, Feb 15, 2010 at 8:18 PM, Robin Anil wrote: > SGD kmeans++ pegasus seems fine. Isabel can you check with the latest trunk > if the perceptron is alright? > I dont see any other open issues which requires patch testing as extensive > as these do > > Robin > > > On Mon, Feb 15, 2010 at 8:10 PM, Drew Farris wrote: > >> On Mon, Feb 15, 2010 at 1:09 AM, Robin Anil wrote: >> > If its A. I have a few patches ready to commit like the static qualifier >> > fix. I really need you guys to be on board on this. We just cant leave >> it at >> > this discussion. >> > >> > If its B. I will do the revert. But would have to patch some commits. >> > >> > If A sounds reasonable. Its easier to go forward than go back. I will >> not be >> > making any more changes at this scale. except bunch of classes from time >> to >> > time. >> >> I think A sounds reasonable, given a patch for MAHOUT-291 that isn't >> as extensive, but I can't really comment on the potential for breaking >> other patches here. I would say that the people with that sort of time >> invested really should have the final say. >> >> Would it make sense for those with outstanding patches to apply 291 >> and then attempt to apply their patches to determine the extent of >> breakage? To be honest, anyone can do it really. If someone wants to >> post some jira issue references for patches that need to be tested I >> can mess around with trying to apply them this evening. >> >> Drew >> > >
Re: Mass Code Cleanup
SGD kmeans++ pegasus seems fine. Isabel can you check with the latest trunk if the perceptron is alright? I dont see any other open issues which requires patch testing as extensive as these do Robin On Mon, Feb 15, 2010 at 8:10 PM, Drew Farris wrote: > On Mon, Feb 15, 2010 at 1:09 AM, Robin Anil wrote: > > If its A. I have a few patches ready to commit like the static qualifier > > fix. I really need you guys to be on board on this. We just cant leave it > at > > this discussion. > > > > If its B. I will do the revert. But would have to patch some commits. > > > > If A sounds reasonable. Its easier to go forward than go back. I will not > be > > making any more changes at this scale. except bunch of classes from time > to > > time. > > I think A sounds reasonable, given a patch for MAHOUT-291 that isn't > as extensive, but I can't really comment on the potential for breaking > other patches here. I would say that the people with that sort of time > invested really should have the final say. > > Would it make sense for those with outstanding patches to apply 291 > and then attempt to apply their patches to determine the extent of > breakage? To be honest, anyone can do it really. If someone wants to > post some jira issue references for patches that need to be tested I > can mess around with trying to apply them this evening. > > Drew >
Re: Mass Code Cleanup
On Mon, Feb 15, 2010 at 1:09 AM, Robin Anil wrote: > If its A. I have a few patches ready to commit like the static qualifier > fix. I really need you guys to be on board on this. We just cant leave it at > this discussion. > > If its B. I will do the revert. But would have to patch some commits. > > If A sounds reasonable. Its easier to go forward than go back. I will not be > making any more changes at this scale. except bunch of classes from time to > time. I think A sounds reasonable, given a patch for MAHOUT-291 that isn't as extensive, but I can't really comment on the potential for breaking other patches here. I would say that the people with that sort of time invested really should have the final say. Would it make sense for those with outstanding patches to apply 291 and then attempt to apply their patches to determine the extent of breakage? To be honest, anyone can do it really. If someone wants to post some jira issue references for patches that need to be tested I can mess around with trying to apply them this evening. Drew
Re: Mass Code Cleanup
In a previous jira issue I had aligned the CS with the lucene style. Its that version which is checked in. Could you try now with ti Robin On Mon, Feb 15, 2010 at 7:44 PM, Benson Margulies wrote: > To answer a question of Robin's: > > Some months ago, I started to make arrangements to include cs in our > build. However, I discovered an aspect of 'Lucene style' that was, at > the time, 100%-incompatible with cs. There was no option to cs to > align it. > > So, the first step here is to agree to a style that cs can, in fact, check. >
Re: Mass Code Cleanup
To answer a question of Robin's: Some months ago, I started to make arrangements to include cs in our build. However, I discovered an aspect of 'Lucene style' that was, at the time, 100%-incompatible with cs. There was no option to cs to align it. So, the first step here is to agree to a style that cs can, in fact, check.
Re: Mass Code Cleanup
On Mon, Feb 15, 2010 at 1:09 AM, Robin Anil wrote: > If its A. I have a few patches ready to commit like the static qualifier > fix. I really need you guys to be on board on this. We just cant leave it at > this discussion. Is there a patch on JIRA for these? It would be easier to review and vote on if there is. Drew
Re: Mass Code Cleanup
If its A. I have a few patches ready to commit like the static qualifier fix. I really need you guys to be on board on this. We just cant leave it at this discussion. If its B. I will do the revert. But would have to patch some commits. If A sounds reasonable. Its easier to go forward than go back. I will not be making any more changes at this scale. except bunch of classes from time to time. Robin On Mon, Feb 15, 2010 at 3:46 AM, Robin Anil wrote: > > I give up. This discussion is really getting to annoy me for some reason. >> >> :) Me too. I was doing this in good will, didnt mean to turn out this way. > If there is any work required in terms of cleaning up. I will be doing it in > the next couple of days. But that entirely depends on the course of action > > A. Current trunk works. Fix the issues with it and stick to the current > lucene code format > B. Revert to Feb12(few small line patches are in especially in utils) > which we can pick and repatch. And leave code as Is? And all of us try to > modify it incrementally to adhere to the lucene code format > > Time is precious as all agree. So choose. > > Robin >
Re: Mass Code Cleanup
On Mon, Feb 15, 2010 at 4:06 AM, Benson Margulies wrote: > > > > I don't object to good style. I object to sweeping changes that break a > lot of patches. Maybe not the case here, but it will be in the future and > unless the whole thing is automated as part of committing (as Hadoop does), > the code will always have formatting issues causing this exact same thing to > happen. I've lived it on Lucene for a lot of years and you can see the > debate in the archives. > > That's why I push putting into the maven build, eclipse, etc. You > don't really need a commit hook if you have that. > > +1 for this. It would be great if you can take this up.
Re: Mass Code Cleanup
> > I don't object to good style. I object to sweeping changes that break a lot > of patches. Maybe not the case here, but it will be in the future and unless > the whole thing is automated as part of committing (as Hadoop does), the code > will always have formatting issues causing this exact same thing to happen. > I've lived it on Lucene for a lot of years and you can see the debate in the > archives. That's why I push putting into the maven build, eclipse, etc. You don't really need a commit hook if you have that. > >> I give up on it though. All I particularly care about is >> the static-initializer thing, and people not complaining about me >> doing small code cleanup. > > I don't have a problem w/ small code cleanup, but that isn't the subject of > this message. I usually reformat the files affected when I commit. IntelliJ > even has this built into it's commit capabilities and I'm all for it. > >> >> On Sun, Feb 14, 2010 at 10:07 PM, Grant Ingersoll >> wrote: >>> If ever there were a case of >>> http://en.wikipedia.org/wiki/Parkinson's_Law_of_Triviality, this is it. >>> Committer time is a scarce resource. Unless it's automated, the code will >>> always drift out of format. I'd rather be able to cleanly apply a patch >>> than worry about a particular style being applied. We can apply the >>> styling when committing. >>> >>> -Grant >>> >>> > > >
Re: Mass Code Cleanup
On Feb 14, 2010, at 5:12 PM, Sean Owen wrote: > Committer time is indeed scarce; nobody is suggesting forcing anyone > to adhere to style. Objecting to others address this is another > matter, and I must say I completely disagree with some thinking I'm > hearing here. I don't object to good style. I object to sweeping changes that break a lot of patches. Maybe not the case here, but it will be in the future and unless the whole thing is automated as part of committing (as Hadoop does), the code will always have formatting issues causing this exact same thing to happen. I've lived it on Lucene for a lot of years and you can see the debate in the archives. > I give up on it though. All I particularly care about is > the static-initializer thing, and people not complaining about me > doing small code cleanup. I don't have a problem w/ small code cleanup, but that isn't the subject of this message. I usually reformat the files affected when I commit. IntelliJ even has this built into it's commit capabilities and I'm all for it. > > On Sun, Feb 14, 2010 at 10:07 PM, Grant Ingersoll wrote: >> If ever there were a case of >> http://en.wikipedia.org/wiki/Parkinson's_Law_of_Triviality, this is it. >> Committer time is a scarce resource. Unless it's automated, the code will >> always drift out of format. I'd rather be able to cleanly apply a patch >> than worry about a particular style being applied. We can apply the styling >> when committing. >> >> -Grant >> >>
Re: Mass Code Cleanup
Over at CXF, we've got a fairly large set of prescriptive rules via PMD and checkstyle. There are some advantages to this. As the number of committers grows, you never have to worry about having to make sense of, for maintenance purposes, some really creative choice of naming, or formatting. We've been operating this way for a long time. It does mean that each of us has accepted some style bits that we don't much like. Are the advantages worth the annoyance? I can't prove it. It may be that the problem domain at CXF is different in a significant way from Mahout. Because the rules are prescriptive (maven and eclipse check them), anyone working on a patch is highly motivated to use the relevant format from the start. Pretty clearly, some of us here at Mahout are not attracted to this mode of operation. If we're just going to treat CS as a tool for checking for bears, and not set the build to enforce, then I think we need a minimal set of rules that don't treat on any toes, and those with time and inclination can make fixes in the least intrusive way they can think of. The sooner we make the fixes, the smaller the number of long-running patches that can get caught across a transition.
Re: Mass Code Cleanup
> I give up. This discussion is really getting to annoy me for some reason. > > :) Me too. I was doing this in good will, didnt mean to turn out this way. If there is any work required in terms of cleaning up. I will be doing it in the next couple of days. But that entirely depends on the course of action A. Current trunk works. Fix the issues with it and stick to the current lucene code format B. Revert to Feb12(few small line patches are in especially in utils) which we can pick and repatch. And leave code as Is? And all of us try to modify it incrementally to adhere to the lucene code format Time is precious as all agree. So choose. Robin
Re: Mass Code Cleanup
Committer time is indeed scarce; nobody is suggesting forcing anyone to adhere to style. Objecting to others address this is another matter, and I must say I completely disagree with some thinking I'm hearing here. I give up on it though. All I particularly care about is the static-initializer thing, and people not complaining about me doing small code cleanup. On Sun, Feb 14, 2010 at 10:07 PM, Grant Ingersoll wrote: > If ever there were a case of > http://en.wikipedia.org/wiki/Parkinson's_Law_of_Triviality, this is it. > Committer time is a scarce resource. Unless it's automated, the code will > always drift out of format. I'd rather be able to cleanly apply a patch than > worry about a particular style being applied. We can apply the styling when > committing. > > -Grant > >
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 2:03 PM, Sean Owen wrote: > Do they even conflict? So much of this is javadoc and line wrap changes. > I don't know if they conflict, but in my patch, there was more than javadocs and line wraps - if I import new stuff, and there was import reordering, svn complains to me about trivial conflicts I have to correct myself. Ditto if I add a method argument, but at the same time, some reordering of modifiers on that method happens. > Would it not be reasonable, per this discussion, to say these patches > ought to be updated to more or less match Lucene style, to the extent > they don't? we've mentioned being more vigilant going forward. > I give up. This discussion is really getting to annoy me for some reason. -jake
Re: Mass Code Cleanup
I've seen auto-formatters make things considerably worse in terms of readability just to make them fit into a particular style. I think that real people will always do a better job at making things readable and adhere to the particular style. I generally agree with Robin's point that now is a good time to go in and clean up by the code due to the low number of outstanding patches prior to a release, but anything entirely automatic gets a -1 from me. Also, for areas where known patches are outstanding, it probably makes sense for the owner/patch author to do the work, at least make the patch conform to the style. I agree that going forward committers should be validating the style of checkins and bouncing patches back to the contributor if they don't conform to the style. If something is important enough to contribute, it is important enough to style it correctly. I suspect the majority of these errors are made out of ignorance as opposed to sloth. That said, it should be nearly effortless (and have a zero time cost) for the committers to validate style using which ever tool they use.
Re: Mass Code Cleanup
These patches dont conflict at all. They dont modify much of the mahout code per say. That do introduce new classes. I am more worried about oliver's changes. on his GIT hub. On Mon, Feb 15, 2010 at 3:29 AM, Jake Mannix wrote: > On Sun, Feb 14, 2010 at 1:53 PM, Sean Owen wrote: > > > I don't see evidence that new code comes in with nearly-correct style, > > nor efforts to correct style in existing code as part of patches -- am > > I missing this? So I'm missing the force that will make this happen, > > short of visible efforts like this, which are being resisted. AFAICT > > it's not going to happen then, and nobody thinks that's good. > > > > I take the point about long-lived patches, but right now we have none > > save perhaps Jake's that stand any chance of being committed. But > > > > Not true: Ted's SGD patch is going in 0.4, as is Zhendong's Pegasos > impl, MAHOUT-279, etc. Anything marked 0.4 has a patch. > > -jake >
Re: Mass Code Cleanup
If ever there were a case of http://en.wikipedia.org/wiki/Parkinson's_Law_of_Triviality, this is it. Committer time is a scarce resource. Unless it's automated, the code will always drift out of format. I'd rather be able to cleanly apply a patch than worry about a particular style being applied. We can apply the styling when committing. -Grant
Re: Mass Code Cleanup
Do they even conflict? So much of this is javadoc and line wrap changes. Would it not be reasonable, per this discussion, to say these patches ought to be updated to more or less match Lucene style, to the extent they don't? we've mentioned being more vigilant going forward. That's exactly what these merge conflicts would be -- highlight exactly the work to be done. This action avoids exactly this problem later -- oh, can you please undo the style fixes you added along with patch X since it broke my patch Y. Yeah, shouldn't have but it'd be really inconvenient to break it in the next few weeks... etc. On Sun, Feb 14, 2010 at 9:59 PM, Jake Mannix wrote: > Not true: Ted's SGD patch is going in 0.4, as is Zhendong's Pegasos > impl, MAHOUT-279, etc. Anything marked 0.4 has a patch. > > -jake >
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 1:53 PM, Sean Owen wrote: > I don't see evidence that new code comes in with nearly-correct style, > nor efforts to correct style in existing code as part of patches -- am > I missing this? So I'm missing the force that will make this happen, > short of visible efforts like this, which are being resisted. AFAICT > it's not going to happen then, and nobody thinks that's good. > > I take the point about long-lived patches, but right now we have none > save perhaps Jake's that stand any chance of being committed. But > Not true: Ted's SGD patch is going in 0.4, as is Zhendong's Pegasos impl, MAHOUT-279, etc. Anything marked 0.4 has a patch. -jake
Re: Mass Code Cleanup
Since nobody will ever call these methods, the one major problem this practice causes goes away. It's always fine IMHO to default to throwing exceptions out of these methods, where they will be logged and handled. Explicitly listing exceptions increases coupling without benefit: if the checked exceptions change, code breaks, when the fix will always be the same and known ahead of time: add it to the method signature. So why bother. I've been 'fixing' this the other way so good to discuss it. On Sun, Feb 14, 2010 at 9:53 PM, Ted Dunning wrote: > Neither of these is necessary. They both can be specific instead of > excessively general. I change this when I see it but don't complain except > when somebody reports to me. >
Re: Mass Code Cleanup
I don't see evidence that new code comes in with nearly-correct style, nor efforts to correct style in existing code as part of patches -- am I missing this? So I'm missing the force that will make this happen, short of visible efforts like this, which are being resisted. AFAICT it's not going to happen then, and nobody thinks that's good. I take the point about long-lived patches, but right now we have none save perhaps Jake's that stand any chance of being committed. But piecewise fixes also have the problem of breaking patches. That's not different. As do any of the API changes bound to happen between 0.2 and 1.0. In such a project, a patch that lives more than a few weeks is bound to accumulate conflicts. Is it so painful to take a small hit once to get 90% there, then sweat the details in due course? Nobody's calling for a hackathon or any active work at all beyond merge pains (which, theoretically, should not have been there in the first place.) Code style is even more important in open source than inside a corporation. Thousands of people need to grok the code as easily as possible, and industry-standard style goes a long way towards that. That said, we all know it'd be harmful to reject contributions and experiments for small reasons of style. But, I am having trouble understanding objections to others taking that work. ... has nobody then noticed the quite large code analysis changes I've been committing? This is hundreds of lines a week. And so far no trouble that I can tell? On Sun, Feb 14, 2010 at 9:36 PM, Grant Ingersoll wrote: > Agreed. We should apply it going forward on those files that have been > touched. We have to be vigilant about what's coming in (Hadoop even goes so > far as to -1 a patch if it doesn't meet the Hadoop style, and all of this is > done automatically) but we shouldn't retrofit. It will cause us committers > way more work in the long run. I've been through this before w/ Lucene and > sweeping code reformatting is a big pain in a multi-committer environment. > There will often be patches that we won't get to for 1, 2 or even 3 releases, > especially as we grow. The easier it is to bring them up to date the better. > > -Grant > >
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 6:53 AM, Sean Owen wrote: > -1 to 'throws Exception' except where harmless or needed: > > - main() > - JUnit > Neither of these is necessary. They both can be specific instead of excessively general. I change this when I see it but don't complain except when somebody reports to me. > - methods which call such unfortunately declared methods > Sad, but true. Even there, I like to catch and rethrow more specific exceptions.
Re: Mass Code Cleanup
On Feb 14, 2010, at 4:25 PM, Jake Mannix wrote: > On Sun, Feb 14, 2010 at 12:16 PM, Benson Margulies > wrote: > >> It seems to me that Robin's campaign is predicated on a prior decision >> as to what checkstyle rules we want to enforce. We can't have it both >> ways. if we want to enforce cs compliance for a ruleset, we've got to >> make the code comply, and one big tornado has something to recommend >> it. If, in fact, we are still in flux in terms of choosing the rules >> we want to enforce, then we have a different problem. >> > > I don't think that deciding to go with one checkstyle choice also means > tha twe "have to" make the code comply. It means that we should be > attempting to make new checkins comply, certainly. Agreed. We should apply it going forward on those files that have been touched. We have to be vigilant about what's coming in (Hadoop even goes so far as to -1 a patch if it doesn't meet the Hadoop style, and all of this is done automatically) but we shouldn't retrofit. It will cause us committers way more work in the long run. I've been through this before w/ Lucene and sweeping code reformatting is a big pain in a multi-committer environment. There will often be patches that we won't get to for 1, 2 or even 3 releases, especially as we grow. The easier it is to bring them up to date the better. -Grant
Re: Mass Code Cleanup
I think we all agree a standard is good, and the standard at the moment is Lucene, and nobody seems against that. I don't think everyone's required to rush out and make a change today or at any particular point, nice as that might be. What's the issue exactly with someone else having a go at fixing 1, 100, or 1000 of the issues? Is it just the work of fixing existing patches, which ought to be at a minimum now? Robin offered to try to take that on too. The discussion could be, do we want a different, more lax standard? But I think that's more effort than just letting this patch go through* and taking the hit more or less once. * modulo the static qualifier business On Sun, Feb 14, 2010 at 9:26 PM, Robin Anil wrote: > I have a patch to remove the StaticQualifier which was bought in due to the > change. But the consensus seem to be in favor of a roll back. In case we do > rollback, what will be the further course of action. I am against leaving > the code untouched. As I said now is the time to do this. If anyone want to > do some addition and substraction from the current checkstyle please do so. > All I am saying is things become much easier If we do this at this point and > not that we shouldnt be liberal in code styling etc > > > On Mon, Feb 15, 2010 at 2:20 AM, Drew Farris wrote: > >> On Sun, Feb 14, 2010 at 2:52 PM, Jake Mannix >> wrote: >> >> > In general, I am a big "-1" to blindly applying the Lucene code >> formatter. >> > It's going to go and do a bunch of stuff like this (unimportant >> reorderings >> > of imports, etc). >> > >> [..] >> >> > I think we've got enough negative votes toward mass application of >> > checkstyle formatting, yes? >> >> Well, if we need one more, I'm a -1 on this too. >> >
Re: Mass Code Cleanup
I have a patch to remove the StaticQualifier which was bought in due to the change. But the consensus seem to be in favor of a roll back. In case we do rollback, what will be the further course of action. I am against leaving the code untouched. As I said now is the time to do this. If anyone want to do some addition and substraction from the current checkstyle please do so. All I am saying is things become much easier If we do this at this point and not that we shouldnt be liberal in code styling etc On Mon, Feb 15, 2010 at 2:20 AM, Drew Farris wrote: > On Sun, Feb 14, 2010 at 2:52 PM, Jake Mannix > wrote: > > > In general, I am a big "-1" to blindly applying the Lucene code > formatter. > > It's going to go and do a bunch of stuff like this (unimportant > reorderings > > of imports, etc). > > > [..] > > > I think we've got enough negative votes toward mass application of > > checkstyle formatting, yes? > > Well, if we need one more, I'm a -1 on this too. >
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 12:16 PM, Benson Margulies wrote: > It seems to me that Robin's campaign is predicated on a prior decision > as to what checkstyle rules we want to enforce. We can't have it both > ways. if we want to enforce cs compliance for a ruleset, we've got to > make the code comply, and one big tornado has something to recommend > it. If, in fact, we are still in flux in terms of choosing the rules > we want to enforce, then we have a different problem. > I don't think that deciding to go with one checkstyle choice also means tha twe "have to" make the code comply. It means that we should be attempting to make new checkins comply, certainly. -jake
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 12:03 PM, Robin Anil wrote: > > > > > > I think we've got enough negative votes toward mass application of > > checkstyle formatting, yes? > > Let me make a case here for having this one time cleanup today. And making > sure we decrease those glaring checkstyle violations. 60-70% of it. With > over 2K violations, I am sure no on is going to fix them any time soon. And > when new code comes the new errors will keep piling with the rest. > So, lets have it now. This mass change fixed the majority. It created some > edge cases which could be hand cleaned. Later we can have some form of > Hackathon where we can fix the rest of it. > But what pain is caused by not having these 2K violations fixed? We're not Google here (or Boeing, where I used to work, where we had svn commit hooks which rejected commits if any file in the commit violated the checkstyle rules), we've got more important things to do than worry about whether there are no "errors" in the checkstyle report. I have no desire to have a Hackathon on fixing coding style. I have a limited amount of my time I can spend on open source programming, and I don't really see much point in spending it changing whitespace and ordering of modifiers and imports. If I wanted to do that, I'd have taken up a job as an editor, not a software engineer. -jake
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 2:52 PM, Jake Mannix wrote: > In general, I am a big "-1" to blindly applying the Lucene code formatter. > It's going to go and do a bunch of stuff like this (unimportant reorderings > of imports, etc). > [..] > I think we've got enough negative votes toward mass application of > checkstyle formatting, yes? Well, if we need one more, I'm a -1 on this too.
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 9:46 AM, Sean Owen wrote: > > I think final is a must on static fields, except in a few exceptional cases. > While it doesn't mean immutable it expresses something useful and makes > static sins harder to commit. > > I feel similarly about instance fields where possible. Instance fields which > hold state must be mutable, but fields which refer to sub-components should > not. A Car should have a mutable fuel level, but its Engine should not be. > > I find these two practices useful enough to suggest they should be enforced. > > I am also a big fan of declaring classes (or more narrowly, certain methods) > final by default. Design for extension or prohibit if says the wise Reverend > Bloch. But this isn't something I'd actively foist on anyone since it needs > judgment about design intent. Chiming in on this late. +1 on all of these points (final on statics, final on non-stateful instance fields, final on classes by default. I first came across this style in lucene many years ago and though it frustrated me at the time, I clearly see the wisdom in doing so). +0 for finals on method parameters. I understand why this is a good idea, but I find that it generally effects readability.
Re: Mass Code Cleanup
Whenever it's come up I recall the consensus being "refer to Lucene checkstyle". There are some JIRAs and threads on it. Seems sensible. I didn't know for instance that the Lucene style had a rule about java.* coming first but that's cool. So this is all right IMHO -- except this static qualifier business. On Sun, Feb 14, 2010 at 8:16 PM, Benson Margulies wrote: > It seems to me that Robin's campaign is predicated on a prior decision > as to what checkstyle rules we want to enforce. We can't have it both > ways. if we want to enforce cs compliance for a ruleset, we've got to > make the code comply, and one big tornado has something to recommend > it. If, in fact, we are still in flux in terms of choosing the rules > we want to enforce, then we have a different problem. >
Re: Mass Code Cleanup
It seems to me that Robin's campaign is predicated on a prior decision as to what checkstyle rules we want to enforce. We can't have it both ways. if we want to enforce cs compliance for a ruleset, we've got to make the code comply, and one big tornado has something to recommend it. If, in fact, we are still in flux in terms of choosing the rules we want to enforce, then we have a different problem. On Sun, Feb 14, 2010 at 3:12 PM, Robin Anil wrote: > More stats. > Major violations types in core examples utils > > mvn checkstyle:checkstyle>checkstyle.txt > cat checkstyle.txt|egrep "core|examples|utils" | cut -d":" -f3-|sed > 's/[0-9]*://g'|sed "s/'[^']*'//g"|sort |uniq -c|sort -rn > > 250 Missing a Javadoc comment. > 92 hides a field. > 90 Name must match pattern . > 54 Line is longer than 120 characters. > 45 is not followed by whitespace. > 40 Constructor definition in wrong order. > 36 Variable access definition in wrong order. > 36 Variable explicitly initialized to (default value for its type). > 30 is not preceded with whitespace. > 21 Wrong order for import. > 21 Variable must be private and have accessor methods. > 18 More than 7 parameters. > 17 Type Javadoc comment is missing an @param tag. > 17 The method should be named . > 17 The method must be declared with no parameters. > 11 Static variable definition in wrong order. >
Re: Mass Code Cleanup
More stats. Major violations types in core examples utils mvn checkstyle:checkstyle>checkstyle.txt cat checkstyle.txt|egrep "core|examples|utils" | cut -d":" -f3-|sed 's/[0-9]*://g'|sed "s/'[^']*'//g"|sort |uniq -c|sort -rn 250 Missing a Javadoc comment. 92 hides a field. 90 Name must match pattern . 54 Line is longer than 120 characters. 45 is not followed by whitespace. 40 Constructor definition in wrong order. 36 Variable access definition in wrong order. 36 Variable explicitly initialized to (default value for its type). 30 is not preceded with whitespace. 21 Wrong order for import. 21 Variable must be private and have accessor methods. 18 More than 7 parameters. 17 Type Javadoc comment is missing an @param tag. 17 The method should be named . 17 The method must be declared with no parameters. 11 Static variable definition in wrong order.
Re: Mass Code Cleanup
> > > I think we've got enough negative votes toward mass application of > checkstyle formatting, yes? Let me make a case here for having this one time cleanup today. And making sure we decrease those glaring checkstyle violations. 60-70% of it. With over 2K violations, I am sure no on is going to fix them any time soon. And when new code comes the new errors will keep piling with the rest. So, lets have it now. This mass change fixed the majority. It created some edge cases which could be hand cleaned. Later we can have some form of Hackathon where we can fix the rest of it. Reverting at this point is pretty easy too. Plus my commits are package by package. So easy to roll back on any particular block without disturbing the rest. Robin
Re: Mass Code Cleanup
If it's 'for a reason' -- meaning this is one big change to follow Lucene convention, I'm down with it. On Sun, Feb 14, 2010 at 7:52 PM, Jake Mannix wrote: > In general, I am a big "-1" to blindly applying the Lucene code formatter. > It's going to go and do a bunch of stuff like this (unimportant reorderings > of imports, etc). >
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 6:46 AM, Sean Owen wrote: > > > I am also a big fan of declaring classes (or more narrowly, certain > methods) > final by default. Design for extension or prohibit if says the wise > Reverend > Bloch. But this isn't something I'd actively foist on anyone since it needs > judgment about design intent. > I second (or third, or fourth) Benson, Ted, and Grant's assertion that in ASF projects, this more often bites us on the ass than helps. > > I actually used to write method params final by default but stopped, even > though I like what it promotes. Nobody is pushing that though. -1 on that from me. Its fine in principal, but it pisses me off to no end when I see a reason to override an open-source project's method that the author didn't realize, and see that I have to work around this by copy/paste or by ripping open the jar and recompiling my own version which removes the final. -jake
Re: Mass Code Cleanup
Some stats on the current trunk v/s previous trunk Checkstyle violations by package after code format 2664 math 739 core 547 collections 149 examples 115 utils 9 taste-web earlier 2664 math 2349 core 938 examples 547 collections 238 utils 9 taste-web Current Checkstyle violators by algorithms 3024 math 295 cf 268 clustering 217 collections 131 df 106 utils 77 common 41 ga 37 fpm 17 classifier 9 text Math is anyways mainly colt style formatting. Disregarding math and collections Top Violating classes 16 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/common/parameters/Parametered.java 14 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/df/tools/FrequenciesJob.java 14 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/dirichlet/DirichletDriver.java 13 /Users/robinanil/mahout-clean/examples/src/main/java/org/apache/mahout/clustering/dirichlet/DisplayDirichlet.java 10 /Users/robinanil/mahout-clean/utils/src/main/java/org/apache/mahout/utils/vectors/lucene/ClusterLabels.java 10 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/df/mapreduce/MapredMapper.java 10 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/fuzzykmeans/SoftCluster.java 10 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/dirichlet/models/L1Model.java 10 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/dirichlet/DirichletCluster.java 10 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/model/jdbc/AbstractJDBCDataModel.java 10 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/common/FastMap.java 9 /Users/robinanil/mahout-clean/examples/src/main/java/org/apache/mahout/clustering/canopy/DisplayCanopy.java 9 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/clustering/kmeans/Cluster.java 9 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/model/GenericBooleanPrefDataModel.java 8 /Users/robinanil/mahout-clean/utils/src/main/java/org/apache/mahout/utils/clustering/ClusterDumper.java 8 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/recommender/slopeone/jdbc/AbstractJDBCDiffStorage.java 8 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/recommender/TreeClusteringRecommender2.java 8 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/recommender/GenericItemBasedRecommender.java 8 /Users/robinanil/mahout-clean/core/src/main/java/org/apache/mahout/cf/taste/impl/common/FastByIDMap.java 7 /Users/robinanil/mahout-clean/utils/src/main/java/org/apache/mahout/utils/vectors/lucene/CachedTermInfo.java On Mon, Feb 15, 2010 at 1:02 AM, Robin Anil wrote: > > On Mon, Feb 15, 2010 at 12:45 AM, Sean Owen wrote: > >> I looked at the result and it seems a fair bit more happened. I don't >> mind line rewrapping in javadoc, or putting newlines after , though >> I do tend to think those are noise changes not worth applying. >> >> I don't know if we ever talked about minor stuff like, do java.* > imports go first or last. But we seemed to have had a convention > going, and every file got reversed. That doesn't seem worthwhile. > > These are part of the Lucene Code formatter. Taken from the Lucene wiki. > Mahout version just cleans up method definitions but sticks to the lucene > version. > > > >> >> But why are all static references qualified now? like, all "log" lines >> are now "MyClass.log(...)" That doesn't seem right to me at all. Lots >> of code is less readable and running well over 120 columns. >> >> return 2.0 * (LogLikelihoodSimilarity.logL(k1 / n1, k1, n1) >> + LogLikelihoodSimilarity.logL(k2 / n2, k2, n2) - >> LogLikelihoodSimilarity.logL(p, k1, n1) - LogLikelihoodSimilarity >>.logL(p, k2, n2)); >> >> > >> This. I am looking. Can be an easier fix than rolling back. >> > > >> I'd propose a rollback just for the last item -- I can't deal with >> that one myself. Then briefly discuss the rest of the changes that >> happened and whether there's consensus. Then hit it again. I do like >> standardization. >> > Take a look at the checkstyle too. If we make that as the > target standardization, then there wont be any issue going forward. If > checkstyle says ok, the code should be ok. > > On Sat, Feb 13, 2010 at 2:55 PM, Robin Anil wrote: >> > I just did a mass code cleanup. >> > >> > Mainly comprising of >> > -Extra blank line removal >> > -Organize Imports across all packages. >> > -Making local variables final >> > >> > No reordering of methods or code style changes are applied. >> > >> > Any objections or any particular class to withhold from committing. >> > >> > Robin >> > >> > >
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 11:32 AM, Robin Anil wrote: > On Mon, Feb 15, 2010 at 12:45 AM, Sean Owen wrote: > > > I looked at the result and it seems a fair bit more happened. I don't > > mind line rewrapping in javadoc, or putting newlines after , though > > I do tend to think those are noise changes not worth applying. > > > > I don't know if we ever talked about minor stuff like, do java.* > imports go first or last. But we seemed to have had a convention > going, and every file got reversed. That doesn't seem worthwhile. > > These are part of the Lucene Code formatter. Taken from the Lucene wiki. > Mahout version just cleans up method definitions but sticks to the lucene > version. In general, I am a big "-1" to blindly applying the Lucene code formatter. It's going to go and do a bunch of stuff like this (unimportant reorderings of imports, etc). > > But why are all static references qualified now? like, all "log" lines > > are now "MyClass.log(...)" That doesn't seem right to me at all. Lots > > of code is less readable and running well over 120 columns. > > > > return 2.0 * (LogLikelihoodSimilarity.logL(k1 / n1, k1, n1) > > + LogLikelihoodSimilarity.logL(k2 / n2, k2, n2) - > > LogLikelihoodSimilarity.logL(p, k1, n1) - LogLikelihoodSimilarity > >.logL(p, k2, n2)); > > > > > > > This. I am looking. Can be an easier fix than rolling back. > Yeah, this I noticed too, and it was pretty annoying. There's no reason for referencing static methods in the same class/subclasses like this. In fact, I actually prefer, in certain specialized cases, to doing the anathema of "import static", when there's a bunch of useful methods or stateless static class instances which are easy to read without the reference (for example: v.assign(otherVector, plus); - pretty obvious what it means, and your IDE can follow in one meta-click to the definition of Functions.plus if you really need to see it). > Take a look at the checkstyle too. If we make that as the > target standardization, then there wont be any issue going forward. If > checkstyle says ok, the code should be ok. > I think we've got enough negative votes toward mass application of checkstyle formatting, yes? -jake
Re: Mass Code Cleanup
On Mon, Feb 15, 2010 at 12:45 AM, Sean Owen wrote: > I looked at the result and it seems a fair bit more happened. I don't > mind line rewrapping in javadoc, or putting newlines after , though > I do tend to think those are noise changes not worth applying. > > I don't know if we ever talked about minor stuff like, do java.* imports go first or last. But we seemed to have had a convention going, and every file got reversed. That doesn't seem worthwhile. These are part of the Lucene Code formatter. Taken from the Lucene wiki. Mahout version just cleans up method definitions but sticks to the lucene version. > > But why are all static references qualified now? like, all "log" lines > are now "MyClass.log(...)" That doesn't seem right to me at all. Lots > of code is less readable and running well over 120 columns. > > return 2.0 * (LogLikelihoodSimilarity.logL(k1 / n1, k1, n1) > + LogLikelihoodSimilarity.logL(k2 / n2, k2, n2) - > LogLikelihoodSimilarity.logL(p, k1, n1) - LogLikelihoodSimilarity >.logL(p, k2, n2)); > > > This. I am looking. Can be an easier fix than rolling back. > > I'd propose a rollback just for the last item -- I can't deal with > that one myself. Then briefly discuss the rest of the changes that > happened and whether there's consensus. Then hit it again. I do like > standardization. > Take a look at the checkstyle too. If we make that as the target standardization, then there wont be any issue going forward. If checkstyle says ok, the code should be ok. On Sat, Feb 13, 2010 at 2:55 PM, Robin Anil wrote: > > I just did a mass code cleanup. > > > > Mainly comprising of > > -Extra blank line removal > > -Organize Imports across all packages. > > -Making local variables final > > > > No reordering of methods or code style changes are applied. > > > > Any objections or any particular class to withhold from committing. > > > > Robin > > >
Re: Mass Code Cleanup
I looked at the result and it seems a fair bit more happened. I don't mind line rewrapping in javadoc, or putting newlines after , though I do tend to think those are noise changes not worth applying. I am not a big fan of the extra parenthesization in boolean expressions, but not totally against it. I don't know if we ever talked about minor stuff like, do java.* imports go first or last. But we seemed to have had a convention going, and every file got reversed. That doesn't seem worthwhile. But why are all static references qualified now? like, all "log" lines are now "MyClass.log(...)" That doesn't seem right to me at all. Lots of code is less readable and running well over 120 columns. return 2.0 * (LogLikelihoodSimilarity.logL(k1 / n1, k1, n1) + LogLikelihoodSimilarity.logL(k2 / n2, k2, n2) - LogLikelihoodSimilarity.logL(p, k1, n1) - LogLikelihoodSimilarity .logL(p, k2, n2)); I'd propose a rollback just for the last item -- I can't deal with that one myself. Then briefly discuss the rest of the changes that happened and whether there's consensus. Then hit it again. I do like standardization. On Sat, Feb 13, 2010 at 2:55 PM, Robin Anil wrote: > I just did a mass code cleanup. > > Mainly comprising of > -Extra blank line removal > -Organize Imports across all packages. > -Making local variables final > > No reordering of methods or code style changes are applied. > > Any objections or any particular class to withhold from committing. > > Robin >
Re: Mass Code Cleanup
On Sun, Feb 14, 2010 at 10:03 AM, Sean Owen wrote: > It would be far too much overhead at this stage to manage every change with > a patch. If in any reasonable doubt about a patch or whether someone would > object, yes open JIRA. > A couple of points. I am +1 to the appropriate use of lazy consensus. I think that I was overly worried about accidentally applying my CXF habits at Mahout. So if some of you have been wondering why I have seemed to be a constant advertisement for my patches, now you know why. I am personally somewhat squeamish about public final methods. To me, there's a sort of Murphy's Law that applies. As soon as you release with a public final method, someone will come up with a legitimate reason to override it. I've been very frustrated in the past by over-enthusiastic finalizers. I have no special opinions about final fields. My comments earlier were to the effect that I don't find them (outside of statics) wildly useful, but I also don't mind them and won't object if Mahout checkstyle/pmd decided to force me to use more of them.
Re: Mass Code Cleanup
It would be far too much overhead at this stage to manage every change with a patch. If in any reasonable doubt about a patch or whether someone would object, yes open JIRA. Otherwise I don't see the harm in leaving it to judgment. We get commit notifications. Changes can be undone. We can tell people committing too freely to let up a bit. That's been my policy. Is that categorically un-Apache or a fine way of operating? On Feb 14, 2010 1:02 PM, "Grant Ingersoll" wrote: On Feb 13, 2010, at 6:37 PM, Benson Margulies wrote: > I'm a little bit confused by the process. >... Nah, we have some laziness in Lucene, too. Trivial cleanup/javadocs, etc. can be committed w/o a patch. To me, it's something you know when you see it.
Re: Mass Code Cleanup
I believe patches shouldn't be so long lived, myself. I think just before a release is a valid time for this. But yeah, that we even observe unconventional code so much perhaps points instead to a failure in coordinating and following standards. I feel strongly about code hygiene as a sort of 'broken windows'-style means of raising the bar in higher level concerns like API design. Google was more militant than I about this and just for the culture it created it was worth it. On Feb 13, 2010 11:24 PM, "Jake Mannix" wrote: On Sat, Feb 13, 2010 at 3:18 PM, Robin Anil wrote: > Sorry about that Jake, ... I guess... but it may come a time in the future when "we have to do this", and I would advocate *not* doing it, actually. It just clutters the subversion history, and can usually be adequately dealt with by accumulating these stylistic fixes over time, IMO. > Robin > PS: I volunteer to help you in getting all the conflicts resolved and > checking it in(O... Thanks, but it's straightforward enough for me to do a whole bunch of "svn merge --dry-run -r BASE:HEAD ." and then working my way through various subdirectories. It's just extra work. I'll try to just get in the habit of checking in my minor changes more frequently so I don't run into this more. -jake
Re: Mass Code Cleanup
-1 to 'throws Exception' except where harmless or needed:
- main()
- JUnit
- methods which call such unfortunately declared methods
There is no good reason a collections class throws IOException in general.
At best its some generic MapException but that doesn't scan.
It's an issue for subclasses to address, as you see them do, by chaining.
RuntimeException is too generic; use perhaps IllegalStateException.
Sean
On Feb 13, 2010 9:20 PM, "Robin Anil" wrote:
On the topic of code cleanup.
Current OpenXXYYhashmaps has to throw runtime exception on an IOException in
Hadoop
This will make that statement clear
void map(Text key, Text value, final OutputCollector output){
forEachPair(function(){
@Override
bool apply(key, value){
try {
do something;
output.collect(key1,value1);
}
catch(IOException e){
throw new RuntimeException(e);
}
return true;
}
}
}
What to do here? Why not let ObjectProcedure class throw Exception ?
On Sun, Feb 14, 2010 at 2:37 AM, Robin Anil wrote: >
final doesnt necess...
Re: Mass Code Cleanup
Sorry for short replies. Tapping this on a phone. I think final is a must on static fields, except in a few exceptional cases. While it doesn't mean immutable it expresses something useful and makes static sins harder to commit. I feel similarly about instance fields where possible. Instance fields which hold state must be mutable, but fields which refer to sub-components should not. A Car should have a mutable fuel level, but its Engine should not be. I find these two practices useful enough to suggest they should be enforced. In fact I have been making such changes regularly. I am also a big fan of declaring classes (or more narrowly, certain methods) final by default. Design for extension or prohibit if says the wise Reverend Bloch. But this isn't something I'd actively foist on anyone since it needs judgment about design intent. I actually used to write method params final by default but stopped, even though I like what it promotes. Nobody is pushing that though. Sean On Feb 13, 2010 9:18 PM, "Ted Dunning" wrote: I find it mildly expressive to be able to say: public static final String KEYWORD_CONSTANT = "foobledy foo"; My reader will know that this is a constant that is safe to use as such. Other than this and the syntactic captured variable case, I don't use final. On Sat, Feb 13, 2010 at 1:05 PM, Benson Margulies wrote: > That doesn't, sadly, prevent > > x[2] = 1; > > So, to me, final is too weak to be useful. I pu... -- Ted Dunning, CTO DeepDyve
Re: Mass Code Cleanup
On Feb 13, 2010, at 6:37 PM, Benson Margulies wrote: > I'm a little bit confused by the process. > > I thought that a hallmark of the Lucene TLP, and the Mahout project > inside of it, was that we do not use lazy consensus. Everything is > offered for review in advance. If at all possible, it is posted as a > patch. Nah, we have some laziness in Lucene, too. Trivial cleanup/javadocs, etc. can be committed w/o a patch. To me, it's something you know when you see it.
Re: Mass Code Cleanup
Whether or not we adhere to patch and review process, I would like it if we did. On Sat, Feb 13, 2010 at 3:37 PM, Benson Margulies wrote: > I thought that a hallmark of the Lucene TLP, and the Mahout project > inside of it, was that we do not use lazy consensus. Everything is > offered for review in advance. If at all possible, it is posted as a > patch. > -- Ted Dunning, CTO DeepDyve
Re: Mass Code Cleanup
On Sat, Feb 13, 2010 at 3:37 PM, Benson Margulies wrote: > I'm a little bit confused by the process. I think you're a little confused because many of us here in Mahout are not also longtime committers on other ASF projects, let alone ones in the Lucene ecosystem. And so things don't always go according to "tradition". Whether it should is another question entirely. -jake
Re: Mass Code Cleanup
In my own opinion, it is a very rare case that I will let the idiom "throws
Exception" appear in any code that I write. It is almost always either a
bug or a bad design.
Letting apply throw Exception almost requires that forEachPair throws the
same excessively general exception. That means that any code that uses this
iteration must either throw Exception or catch Exception and try to sort it
out with instanceof drecky code. I can imagine it is OK for a framework to
internally include something horrid in the name of generality, but not if it
forces me to include that same goofiness in my own code.
So I would be strongly -1 on defining apply to throw Exception. The code
you have here is a small pain the in the posterior to write, but it will
generally get written automagically by the IDE. It has the great virtue of
meaning what it says.
One other option that I could imagine would be to let forEachPair be somehow
genericized to tell it what kind of exception that apply might throw.
Something like new Function() { bool apply(key,value) throws
LegalException {...} } might work. Making this actually usable would
require lots of thought.
On Sat, Feb 13, 2010 at 1:20 PM, Robin Anil wrote:
> forEachPair(function(){
> @Override
> bool apply(key, value){
>try {
> do something;
> output.collect(key1,value1);
>}
>catch(IOException e){
>throw new RuntimeException(e);
>}
>return true;
> }
> }
>
> }
>
>
> What to do here? Why not let ObjectProcedure class throw Exception ?
>
--
Ted Dunning, CTO
DeepDyve
Re: Mass Code Cleanup
I'm a little bit confused by the process. I thought that a hallmark of the Lucene TLP, and the Mahout project inside of it, was that we do not use lazy consensus. Everything is offered for review in advance. If at all possible, it is posted as a patch. If something can't be posted as a patch on a JIRA, I'd expect to see some conspicuous email making the proposal. Maybe I missed some email, I'm not reading this list as religiously as I might. Over at CXF, where we do use lazy consensus, we still have a rough convention of posting a proposal and collecting feedback before making a pervasive change. I am a straddler of the fence here. From time to time, IMHO, reasons come up to make a whole lot of changes. And when they do come up, it's important to coordinate. Please pardon my soapboxing here.
Re: Mass Code Cleanup
On Sat, Feb 13, 2010 at 3:18 PM, Robin Anil wrote: > Sorry about that Jake, At one point we had to do this. My feeling was that, > this was a better time(with most of the issues closed and all) to this, and > would have proved impossible later. > I guess... but it may come a time in the future when "we have to do this", and I would advocate *not* doing it, actually. It just clutters the subversion history, and can usually be adequately dealt with by accumulating these stylistic fixes over time, IMO. > Robin > PS: I volunteer to help you in getting all the conflicts resolved and > checking it in(Offer goes only to the open issues with active work at the > moment) > Thanks, but it's straightforward enough for me to do a whole bunch of "svn merge --dry-run -r BASE:HEAD ." and then working my way through various subdirectories. It's just extra work. I'll try to just get in the habit of checking in my minor changes more frequently so I don't run into this more. -jake
Re: Mass Code Cleanup
Sorry about that Jake, At one point we had to do this. My feeling was that, this was a better time(with most of the issues closed and all) to this, and would have proved impossible later. Robin PS: I volunteer to help you in getting all the conflicts resolved and checking it in(Offer goes only to the open issues with active work at the moment)
Re: Mass Code Cleanup
I've got to say, in the future, I really agree with Grant - we can't go and do mass code changes, we've always got little experimental patches and checkouts going on, and it's hard enough making sure to keep all of those svn up'ed as needed, let alone worry about sudden piles of incoming conflicts do to this kind of thing. Maybe if I checked my code in faster, this wouldn't be quite as annoying, but I'd hate to feel like I should be racing to get my stuff in before it gets clobbered... -jake On Sat, Feb 13, 2010 at 2:34 PM, Robin Anil wrote: > I have mahout math, mahout collections and core/o.a.m.math and > core/o.a.m.classifier.discriminative left to commit. I am assuming that > could break the sgd, winnow and svm classifier and the svd. > > So I am waiting for the svd to come it before doing anything more > > > Robin >
Re: Mass Code Cleanup
I have mahout math, mahout collections and core/o.a.m.math and core/o.a.m.classifier.discriminative left to commit. I am assuming that could break the sgd, winnow and svm classifier and the svd. So I am waiting for the svd to come it before doing anything more Robin
Re: Mass Code Cleanup
In our particular case. Hadoop mapreduce creates the issue
On Sun, Feb 14, 2010 at 3:25 AM, Benson Margulies wrote:
> Ted: I agree. I forgot about that case.
>
> Robin: There is a strange general habit in Java of this situation.
> Consider Thread. I'm perfectly happy to add the throws clause, but one
> might wonder why the 'classic' examples all have the problem you cite.
>
>
> On Sat, Feb 13, 2010 at 4:20 PM, Robin Anil wrote:
> > On the topic of code cleanup.
> >
> > Current OpenXXYYhashmaps has to throw runtime exception on an IOException
> in
> > Hadoop
> >
> > This will make that statement clear
> >
> > void map(Text key, Text value, final OutputCollector output){
> >
> > forEachPair(function(){
> > @Override
> > bool apply(key, value){
> >try {
> > do something;
> > output.collect(key1,value1);
> >}
> >catch(IOException e){
> >throw new RuntimeException(e);
> >}
> >return true;
> > }
> > }
> >
> > }
> >
> >
> > What to do here? Why not let ObjectProcedure class throw Exception ?
> >
> >
> >
> >
> > On Sun, Feb 14, 2010 at 2:37 AM, Robin Anil
> wrote:
> >
> >> final doesnt necessarily mean mutable right?
> >>
> >>
> >> On Sun, Feb 14, 2010 at 2:35 AM, Benson Margulies <
> bimargul...@gmail.com>wrote:
> >>
> >>> I'm not very fond of a plague of finals. Here's why.
> >>>
> >>> Consider
> >>>
> >>> final int[] x = new int[10];
> >>>
> >>> That doesn't, sadly, prevent
> >>>
> >>>x[2] = 1;
> >>>
> >>> So, to me, final is too weak to be useful. I put them in code when
> >>> required due to the rules about anonymous functions capturing locals,
> >>> but never otherwise.
> >>>
> >>> Just my 1cent on this subject.
> >>>
> >>>
> >>> On Sat, Feb 13, 2010 at 3:21 PM, Robin Anil
> wrote:
> >>> > Only done for local fields. And handpicked ones in bayes for local
> >>> variables
> >>> > due to openXXYYHashMap.foreach() requiring final objects
> >>> >
> >>> >
> >>> > On Sun, Feb 14, 2010 at 1:47 AM, Ted Dunning
> >>> wrote:
> >>> >
> >>> >> I find final slightly helpful on fields, very helpful on static
> fields,
> >>> but
> >>> >> not very helpful at all on local variables. The issue is scale.
> With
> >>> a
> >>> >> field there are lots of places you can bugger it up. If the scope
> of a
> >>> >> local is large enough for similar confusion, you have a different
> >>> problem
> >>> >> anyway.
> >>> >>
> >>> >> I am generally -1 to too many final declarations on parameters and
> >>> local
> >>> >> variables, but I don't go to the trouble of deleting them if I see
> >>> them.
> >>> >>
> >>> >> On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen
> wrote:
> >>> >>
> >>> >> > I'm late on this but have a question about the final business. I
> >>> >> understand
> >>> >> > the style it is promoting and even like it and used to do it. I
> >>> stopped
> >>> >> > because it does get harder to read and its not usual in java code.
> >>> Any
> >>> >> > thoughts on that.
> >>> >> >
> >>> >> > On Feb 13, 2010 2:55 PM, "Robin Anil"
> wrote:
> >>> >> >
> >>> >> > I just did a mass code cleanup.
> >>> >> >
> >>> >> > Mainly comprising of
> >>> >> > -Extra blank line removal
> >>> >> > -Organize Imports across all packages.
> >>> >> > -Making local variables final
> >>> >> >
> >>> >> > No reordering of methods or code style changes are applied.
> >>> >> >
> >>> >> > Any objections or any particular class to withhold from
> committing.
> >>> >> >
> >>> >> > Robin
> >>> >> >
> >>> >>
> >>> >>
> >>> >>
> >>> >> --
> >>> >> Ted Dunning, CTO
> >>> >> DeepDyve
> >>> >>
> >>> >
> >>>
> >>
> >>
> >
>
Re: Mass Code Cleanup
Ted: I agree. I forgot about that case.
Robin: There is a strange general habit in Java of this situation.
Consider Thread. I'm perfectly happy to add the throws clause, but one
might wonder why the 'classic' examples all have the problem you cite.
On Sat, Feb 13, 2010 at 4:20 PM, Robin Anil wrote:
> On the topic of code cleanup.
>
> Current OpenXXYYhashmaps has to throw runtime exception on an IOException in
> Hadoop
>
> This will make that statement clear
>
> void map(Text key, Text value, final OutputCollector output){
>
> forEachPair(function(){
> �...@override
> bool apply(key, value){
> try {
> do something;
> output.collect(key1,value1);
> }
> catch(IOException e){
> throw new RuntimeException(e);
> }
> return true;
> }
> }
>
> }
>
>
> What to do here? Why not let ObjectProcedure class throw Exception ?
>
>
>
>
> On Sun, Feb 14, 2010 at 2:37 AM, Robin Anil wrote:
>
>> final doesnt necessarily mean mutable right?
>>
>>
>> On Sun, Feb 14, 2010 at 2:35 AM, Benson Margulies
>> wrote:
>>
>>> I'm not very fond of a plague of finals. Here's why.
>>>
>>> Consider
>>>
>>> final int[] x = new int[10];
>>>
>>> That doesn't, sadly, prevent
>>>
>>> x[2] = 1;
>>>
>>> So, to me, final is too weak to be useful. I put them in code when
>>> required due to the rules about anonymous functions capturing locals,
>>> but never otherwise.
>>>
>>> Just my 1cent on this subject.
>>>
>>>
>>> On Sat, Feb 13, 2010 at 3:21 PM, Robin Anil wrote:
>>> > Only done for local fields. And handpicked ones in bayes for local
>>> variables
>>> > due to openXXYYHashMap.foreach() requiring final objects
>>> >
>>> >
>>> > On Sun, Feb 14, 2010 at 1:47 AM, Ted Dunning
>>> wrote:
>>> >
>>> >> I find final slightly helpful on fields, very helpful on static fields,
>>> but
>>> >> not very helpful at all on local variables. The issue is scale. With
>>> a
>>> >> field there are lots of places you can bugger it up. If the scope of a
>>> >> local is large enough for similar confusion, you have a different
>>> problem
>>> >> anyway.
>>> >>
>>> >> I am generally -1 to too many final declarations on parameters and
>>> local
>>> >> variables, but I don't go to the trouble of deleting them if I see
>>> them.
>>> >>
>>> >> On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen wrote:
>>> >>
>>> >> > I'm late on this but have a question about the final business. I
>>> >> understand
>>> >> > the style it is promoting and even like it and used to do it. I
>>> stopped
>>> >> > because it does get harder to read and its not usual in java code.
>>> Any
>>> >> > thoughts on that.
>>> >> >
>>> >> > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote:
>>> >> >
>>> >> > I just did a mass code cleanup.
>>> >> >
>>> >> > Mainly comprising of
>>> >> > -Extra blank line removal
>>> >> > -Organize Imports across all packages.
>>> >> > -Making local variables final
>>> >> >
>>> >> > No reordering of methods or code style changes are applied.
>>> >> >
>>> >> > Any objections or any particular class to withhold from committing.
>>> >> >
>>> >> > Robin
>>> >> >
>>> >>
>>> >>
>>> >>
>>> >> --
>>> >> Ted Dunning, CTO
>>> >> DeepDyve
>>> >>
>>> >
>>>
>>
>>
>
Re: Mass Code Cleanup
On the topic of code cleanup.
Current OpenXXYYhashmaps has to throw runtime exception on an IOException in
Hadoop
This will make that statement clear
void map(Text key, Text value, final OutputCollector output){
forEachPair(function(){
@Override
bool apply(key, value){
try {
do something;
output.collect(key1,value1);
}
catch(IOException e){
throw new RuntimeException(e);
}
return true;
}
}
}
What to do here? Why not let ObjectProcedure class throw Exception ?
On Sun, Feb 14, 2010 at 2:37 AM, Robin Anil wrote:
> final doesnt necessarily mean mutable right?
>
>
> On Sun, Feb 14, 2010 at 2:35 AM, Benson Margulies
> wrote:
>
>> I'm not very fond of a plague of finals. Here's why.
>>
>> Consider
>>
>> final int[] x = new int[10];
>>
>> That doesn't, sadly, prevent
>>
>>x[2] = 1;
>>
>> So, to me, final is too weak to be useful. I put them in code when
>> required due to the rules about anonymous functions capturing locals,
>> but never otherwise.
>>
>> Just my 1cent on this subject.
>>
>>
>> On Sat, Feb 13, 2010 at 3:21 PM, Robin Anil wrote:
>> > Only done for local fields. And handpicked ones in bayes for local
>> variables
>> > due to openXXYYHashMap.foreach() requiring final objects
>> >
>> >
>> > On Sun, Feb 14, 2010 at 1:47 AM, Ted Dunning
>> wrote:
>> >
>> >> I find final slightly helpful on fields, very helpful on static fields,
>> but
>> >> not very helpful at all on local variables. The issue is scale. With
>> a
>> >> field there are lots of places you can bugger it up. If the scope of a
>> >> local is large enough for similar confusion, you have a different
>> problem
>> >> anyway.
>> >>
>> >> I am generally -1 to too many final declarations on parameters and
>> local
>> >> variables, but I don't go to the trouble of deleting them if I see
>> them.
>> >>
>> >> On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen wrote:
>> >>
>> >> > I'm late on this but have a question about the final business. I
>> >> understand
>> >> > the style it is promoting and even like it and used to do it. I
>> stopped
>> >> > because it does get harder to read and its not usual in java code.
>> Any
>> >> > thoughts on that.
>> >> >
>> >> > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote:
>> >> >
>> >> > I just did a mass code cleanup.
>> >> >
>> >> > Mainly comprising of
>> >> > -Extra blank line removal
>> >> > -Organize Imports across all packages.
>> >> > -Making local variables final
>> >> >
>> >> > No reordering of methods or code style changes are applied.
>> >> >
>> >> > Any objections or any particular class to withhold from committing.
>> >> >
>> >> > Robin
>> >> >
>> >>
>> >>
>> >>
>> >> --
>> >> Ted Dunning, CTO
>> >> DeepDyve
>> >>
>> >
>>
>
>
Re: Mass Code Cleanup
I find it mildly expressive to be able to say: public static final String KEYWORD_CONSTANT = "foobledy foo"; My reader will know that this is a constant that is safe to use as such. Other than this and the syntactic captured variable case, I don't use final. On Sat, Feb 13, 2010 at 1:05 PM, Benson Margulies wrote: > That doesn't, sadly, prevent > >x[2] = 1; > > So, to me, final is too weak to be useful. I put them in code when > required due to the rules about anonymous functions capturing locals, > but never otherwise. > -- Ted Dunning, CTO DeepDyve
Re: Mass Code Cleanup
You meant immutable? I wouldn't disagree. I appreciate what it does, I just don't much like it :-) On Sat, Feb 13, 2010 at 4:07 PM, Robin Anil wrote: > final doesnt necessarily mean mutable right? > > > On Sun, Feb 14, 2010 at 2:35 AM, Benson Margulies > wrote: > >> I'm not very fond of a plague of finals. Here's why. >> >> Consider >> >> final int[] x = new int[10]; >> >> That doesn't, sadly, prevent >> >> x[2] = 1; >> >> So, to me, final is too weak to be useful. I put them in code when >> required due to the rules about anonymous functions capturing locals, >> but never otherwise. >> >> Just my 1cent on this subject. >> >> >> On Sat, Feb 13, 2010 at 3:21 PM, Robin Anil wrote: >> > Only done for local fields. And handpicked ones in bayes for local >> variables >> > due to openXXYYHashMap.foreach() requiring final objects >> > >> > >> > On Sun, Feb 14, 2010 at 1:47 AM, Ted Dunning >> wrote: >> > >> >> I find final slightly helpful on fields, very helpful on static fields, >> but >> >> not very helpful at all on local variables. The issue is scale. With a >> >> field there are lots of places you can bugger it up. If the scope of a >> >> local is large enough for similar confusion, you have a different >> problem >> >> anyway. >> >> >> >> I am generally -1 to too many final declarations on parameters and local >> >> variables, but I don't go to the trouble of deleting them if I see them. >> >> >> >> On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen wrote: >> >> >> >> > I'm late on this but have a question about the final business. I >> >> understand >> >> > the style it is promoting and even like it and used to do it. I >> stopped >> >> > because it does get harder to read and its not usual in java code. Any >> >> > thoughts on that. >> >> > >> >> > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote: >> >> > >> >> > I just did a mass code cleanup. >> >> > >> >> > Mainly comprising of >> >> > -Extra blank line removal >> >> > -Organize Imports across all packages. >> >> > -Making local variables final >> >> > >> >> > No reordering of methods or code style changes are applied. >> >> > >> >> > Any objections or any particular class to withhold from committing. >> >> > >> >> > Robin >> >> > >> >> >> >> >> >> >> >> -- >> >> Ted Dunning, CTO >> >> DeepDyve >> >> >> > >> >
Re: Mass Code Cleanup
final doesnt necessarily mean mutable right? On Sun, Feb 14, 2010 at 2:35 AM, Benson Margulies wrote: > I'm not very fond of a plague of finals. Here's why. > > Consider > > final int[] x = new int[10]; > > That doesn't, sadly, prevent > >x[2] = 1; > > So, to me, final is too weak to be useful. I put them in code when > required due to the rules about anonymous functions capturing locals, > but never otherwise. > > Just my 1cent on this subject. > > > On Sat, Feb 13, 2010 at 3:21 PM, Robin Anil wrote: > > Only done for local fields. And handpicked ones in bayes for local > variables > > due to openXXYYHashMap.foreach() requiring final objects > > > > > > On Sun, Feb 14, 2010 at 1:47 AM, Ted Dunning > wrote: > > > >> I find final slightly helpful on fields, very helpful on static fields, > but > >> not very helpful at all on local variables. The issue is scale. With a > >> field there are lots of places you can bugger it up. If the scope of a > >> local is large enough for similar confusion, you have a different > problem > >> anyway. > >> > >> I am generally -1 to too many final declarations on parameters and local > >> variables, but I don't go to the trouble of deleting them if I see them. > >> > >> On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen wrote: > >> > >> > I'm late on this but have a question about the final business. I > >> understand > >> > the style it is promoting and even like it and used to do it. I > stopped > >> > because it does get harder to read and its not usual in java code. Any > >> > thoughts on that. > >> > > >> > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote: > >> > > >> > I just did a mass code cleanup. > >> > > >> > Mainly comprising of > >> > -Extra blank line removal > >> > -Organize Imports across all packages. > >> > -Making local variables final > >> > > >> > No reordering of methods or code style changes are applied. > >> > > >> > Any objections or any particular class to withhold from committing. > >> > > >> > Robin > >> > > >> > >> > >> > >> -- > >> Ted Dunning, CTO > >> DeepDyve > >> > > >
Re: Mass Code Cleanup
I'm not very fond of a plague of finals. Here's why. Consider final int[] x = new int[10]; That doesn't, sadly, prevent x[2] = 1; So, to me, final is too weak to be useful. I put them in code when required due to the rules about anonymous functions capturing locals, but never otherwise. Just my 1cent on this subject. On Sat, Feb 13, 2010 at 3:21 PM, Robin Anil wrote: > Only done for local fields. And handpicked ones in bayes for local variables > due to openXXYYHashMap.foreach() requiring final objects > > > On Sun, Feb 14, 2010 at 1:47 AM, Ted Dunning wrote: > >> I find final slightly helpful on fields, very helpful on static fields, but >> not very helpful at all on local variables. The issue is scale. With a >> field there are lots of places you can bugger it up. If the scope of a >> local is large enough for similar confusion, you have a different problem >> anyway. >> >> I am generally -1 to too many final declarations on parameters and local >> variables, but I don't go to the trouble of deleting them if I see them. >> >> On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen wrote: >> >> > I'm late on this but have a question about the final business. I >> understand >> > the style it is promoting and even like it and used to do it. I stopped >> > because it does get harder to read and its not usual in java code. Any >> > thoughts on that. >> > >> > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote: >> > >> > I just did a mass code cleanup. >> > >> > Mainly comprising of >> > -Extra blank line removal >> > -Organize Imports across all packages. >> > -Making local variables final >> > >> > No reordering of methods or code style changes are applied. >> > >> > Any objections or any particular class to withhold from committing. >> > >> > Robin >> > >> >> >> >> -- >> Ted Dunning, CTO >> DeepDyve >> >
Re: Mass Code Cleanup
Only done for local fields. And handpicked ones in bayes for local variables due to openXXYYHashMap.foreach() requiring final objects On Sun, Feb 14, 2010 at 1:47 AM, Ted Dunning wrote: > I find final slightly helpful on fields, very helpful on static fields, but > not very helpful at all on local variables. The issue is scale. With a > field there are lots of places you can bugger it up. If the scope of a > local is large enough for similar confusion, you have a different problem > anyway. > > I am generally -1 to too many final declarations on parameters and local > variables, but I don't go to the trouble of deleting them if I see them. > > On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen wrote: > > > I'm late on this but have a question about the final business. I > understand > > the style it is promoting and even like it and used to do it. I stopped > > because it does get harder to read and its not usual in java code. Any > > thoughts on that. > > > > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote: > > > > I just did a mass code cleanup. > > > > Mainly comprising of > > -Extra blank line removal > > -Organize Imports across all packages. > > -Making local variables final > > > > No reordering of methods or code style changes are applied. > > > > Any objections or any particular class to withhold from committing. > > > > Robin > > > > > > -- > Ted Dunning, CTO > DeepDyve >
Re: Mass Code Cleanup
I find final slightly helpful on fields, very helpful on static fields, but not very helpful at all on local variables. The issue is scale. With a field there are lots of places you can bugger it up. If the scope of a local is large enough for similar confusion, you have a different problem anyway. I am generally -1 to too many final declarations on parameters and local variables, but I don't go to the trouble of deleting them if I see them. On Sat, Feb 13, 2010 at 11:03 AM, Sean Owen wrote: > I'm late on this but have a question about the final business. I understand > the style it is promoting and even like it and used to do it. I stopped > because it does get harder to read and its not usual in java code. Any > thoughts on that. > > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote: > > I just did a mass code cleanup. > > Mainly comprising of > -Extra blank line removal > -Organize Imports across all packages. > -Making local variables final > > No reordering of methods or code style changes are applied. > > Any objections or any particular class to withhold from committing. > > Robin > -- Ted Dunning, CTO DeepDyve
Re: Mass Code Cleanup
Not everything is changed. Only at a few places. On Sun, Feb 14, 2010 at 12:42 AM, Sean Owen wrote: > Declaring every local variable final. > > On Feb 13, 2010 7:05 PM, "Robin Anil" wrote: > > Which part is not usual? > > On Sun, Feb 14, 2010 at 12:33 AM, Sean Owen wrote: > > I'm > late on this but hav... >
Re: Mass Code Cleanup
Declaring every local variable final. On Feb 13, 2010 7:05 PM, "Robin Anil" wrote: Which part is not usual? On Sun, Feb 14, 2010 at 12:33 AM, Sean Owen wrote: > I'm late on this but hav...
Re: Mass Code Cleanup
Which part is not usual? On Sun, Feb 14, 2010 at 12:33 AM, Sean Owen wrote: > I'm late on this but have a question about the final business. I understand > the style it is promoting and even like it and used to do it. I stopped > because it does get harder to read and its not usual in java code. Any > thoughts on that. > > On Feb 13, 2010 2:55 PM, "Robin Anil" wrote: > > I just did a mass code cleanup. > > Mainly comprising of > -Extra blank line removal > -Organize Imports across all packages. > -Making local variables final > > No reordering of methods or code style changes are applied. > > Any objections or any particular class to withhold from committing. > > Robin >
Re: Mass Code Cleanup
I'm late on this but have a question about the final business. I understand the style it is promoting and even like it and used to do it. I stopped because it does get harder to read and its not usual in java code. Any thoughts on that. On Feb 13, 2010 2:55 PM, "Robin Anil" wrote: I just did a mass code cleanup. Mainly comprising of -Extra blank line removal -Organize Imports across all packages. -Making local variables final No reordering of methods or code style changes are applied. Any objections or any particular class to withhold from committing. Robin
Re: Mass Code Cleanup
I hope my svd patch isn't destroyed... I guess I'll see later when I have time to check... -jake On Feb 13, 2010 10:00 AM, "Robin Anil" wrote: Utils done. Svn up On Sat, Feb 13, 2010 at 10:23 PM, Robin Anil wrote: > Yeah I thought so. ...
Re: Mass Code Cleanup
Utils done. Svn up On Sat, Feb 13, 2010 at 10:23 PM, Robin Anil wrote: > Yeah I thought so. We are just before a release. And very few issues are > left with patches submitted. Thats why I wanted that list of files so that I > can commit the rest > > > > > On Sat, Feb 13, 2010 at 10:21 PM, Grant Ingersoll wrote: > >> It's fine to go with this one, but in general massive reformatting is not >> necessarily a good thing b/c it likely breaks a whole bunch of perfectly >> valid patches in JIRA, thus making more work for you later, not less. >> Instead, I think the way to handle this stuff is to make sure that, when >> ready to commit, all of those files that have been touched are properly >> formatted. This will still likely break some patches, but not as many. >> >> On Feb 13, 2010, at 9:55 AM, Robin Anil wrote: >> >> > I just did a mass code cleanup. >> > >> > Mainly comprising of >> > -Extra blank line removal >> > -Organize Imports across all packages. >> > -Making local variables final >> > >> > No reordering of methods or code style changes are applied. >> > >> > Any objections or any particular class to withhold from committing. >> > >> > Robin >> >> >> >
Re: Mass Code Cleanup
Yeah I thought so. We are just before a release. And very few issues are left with patches submitted. Thats why I wanted that list of files so that I can commit the rest On Sat, Feb 13, 2010 at 10:21 PM, Grant Ingersoll wrote: > It's fine to go with this one, but in general massive reformatting is not > necessarily a good thing b/c it likely breaks a whole bunch of perfectly > valid patches in JIRA, thus making more work for you later, not less. > Instead, I think the way to handle this stuff is to make sure that, when > ready to commit, all of those files that have been touched are properly > formatted. This will still likely break some patches, but not as many. > > On Feb 13, 2010, at 9:55 AM, Robin Anil wrote: > > > I just did a mass code cleanup. > > > > Mainly comprising of > > -Extra blank line removal > > -Organize Imports across all packages. > > -Making local variables final > > > > No reordering of methods or code style changes are applied. > > > > Any objections or any particular class to withhold from committing. > > > > Robin > > >
Re: Mass Code Cleanup
It's fine to go with this one, but in general massive reformatting is not necessarily a good thing b/c it likely breaks a whole bunch of perfectly valid patches in JIRA, thus making more work for you later, not less. Instead, I think the way to handle this stuff is to make sure that, when ready to commit, all of those files that have been touched are properly formatted. This will still likely break some patches, but not as many. On Feb 13, 2010, at 9:55 AM, Robin Anil wrote: > I just did a mass code cleanup. > > Mainly comprising of > -Extra blank line removal > -Organize Imports across all packages. > -Making local variables final > > No reordering of methods or code style changes are applied. > > Any objections or any particular class to withhold from committing. > > Robin
Re: Mass Code Cleanup
And yes all tests pass. No problems there. Robin On Sat, Feb 13, 2010 at 8:25 PM, Robin Anil wrote: > I just did a mass code cleanup. > > Mainly comprising of > -Extra blank line removal > -Organize Imports across all packages. > -Making local variables final > > No reordering of methods or code style changes are applied. > > Any objections or any particular class to withhold from committing. > > Robin > > > >
Mass Code Cleanup
I just did a mass code cleanup. Mainly comprising of -Extra blank line removal -Organize Imports across all packages. -Making local variables final No reordering of methods or code style changes are applied. Any objections or any particular class to withhold from committing. Robin
