accumulo icon indicating copy to clipboard operation
accumulo copied to clipboard

Remove TODOs

Open ctubbsii opened this issue 2 years ago • 2 comments

There's a bunch of TODOs in code now. We previously had them all removed, in favor of creating tickets to track work. I guess we've been less rigorous about that recently.

All the existing TODOs should be checked for relevancy and deleted. If they are still relevant and an issue does not yet exist for them, then the details should be put into a new issue for proper tracking.

I understand that the "TODO" keyword may be helpful in some IDEs that report such things, but it does more harm than help to have issues tracked in multiple places, and here is where the project has decided to track issues, because we can't make reliable assumptions about what IDE (if any) a contributor might be using. Some IDEs have plugins that allow them to show issues (Eclipse has Mylyn, for example) if one wishes to see them in their IDE.

ctubbsii avatar May 12 '22 20:05 ctubbsii

I was looking through some of the TODOs and came across one in PerTableVolumeChooser.java: https://github.com/apache/accumulo/blob/d3a02c9ccb79548caf037d27aa9b60c2a6c6b698/core/src/main/java/org/apache/accumulo/core/spi/fs/PerTableVolumeChooser.java#L40

The TODO discusses the changing of the name of the PerTableVolumeChooser class due to a concern with the current class name and the overall accuracy of what it is doing.

I wanted to put a comment on this issue to see if this TODO is still relevant, and has a need to be changed, or if it could be safely removed.

foster33 avatar May 23 '22 16:05 foster33

I was looking through some of the TODOs and came across one in PerTableVolumeChooser.java: I wanted to put a comment on this issue to see if this TODO is still relevant, and has a need to be changed, or if it could be safely removed.

The file name can be changed, both TODOs can be removed (the second one isn't necessary to do) and the typo in the class level javadoc: "At the this this was written" can be fixed to "At the time this was written". Feel free to make a PR for this.

ctubbsii avatar May 23 '22 23:05 ctubbsii