Prasanna, More irksome than forgetting to remove TODO comments after addressing them are TODOs that actually are TODOs and not cruft. The ticket referenced below us the later. I am fine with having an IDE generate them as a reminder to finish something. Contributors need to remember to address and remove them before submission because I see them take at face value -- there is more work to be done.
Thanks, -John On Jun 28, 2013, at 12:08 PM, Prasanna Santhanam <t...@apache.org> wrote: > +1 > > The TODOs, @author tags and minor typos irks me too. Typos are > understandable and I fix them when I see them. But it we should be > keep the code clean of IDE introduced comments like TODOs. Please > consider configuring your IDEs. > > Let's all _care_ for our codebase ... just a little. > > Now everyone go clean your classes :) > > On Fri, Jun 28, 2013 at 10:06:47AM -0400, John Burwell wrote: >> All, >> >> While reviewing the solidfire patch, I noticed that a dummy >> getter/setter implementations with TODO comments in the >> PrimaryDataStoreImpl in were merged into master. I have opened a >> defect [1] for this issue, as the code will not work as expected >> (they are not actually working with the internal state of the object >> as expected). >> >> Speaking generally, every review I have performed for 4.2.0 has >> contained TODO comments, as well, tab characters and improper >> indentation. Please check patches for these issues before >> submission. For me, patches with TODOs are an immediate -1 because >> it conveys that the implementation is incomplete. The formatting >> issues are minor, but easily rectified with a quick code format in >> an IDE. >> >> Thanks, >> -John >> >> [1]: https://issues.apache.org/jira/browse/CLOUDSTACK-3277 > > -- > Prasanna., > > ------------------------ > Powered by BigRock.com >
