Jon Haddad commented on CASSANDRA-13010:

Hey [~alourie] I just took a look at the patch.  There's still a pretty big 
(200 LOC) conflict between trunk and what's in your branch.  Would you mind 
rebasing your branch off trunk so it applies cleanly?  According to the git 
history you merged in some changes from trunk, which are now making a lot 
harder to do the review, as I have to look through the history to determine 
what's actually been deleted and what's a change you made.  For instance it 
looks like you deleted {{doValidationCompaction}}, which you didn't, Blake 
Eggleston did in {{ c5a7fcaa8e000}}.

A few other notes while I'm in here to avoid lots of iterations:

# there's almost no comments added despite it touching almost 20 files.  I 
realize it's not the best commented codebase, but I'd like to see comments on 
any new variables like {{targetDirectory}}.  Specifically, consider why 
something is there {{// needed for nodetool compactioninfo output}} is better 
than {{// holds directory name}}.  Please add comments conveying intent for 
each class method and variable added.
# {{import org.apache.cassandra.cql3.Operation}} was added as an import to 
but not used

Outside that, I think it's looking pretty good, I'll be pretty happy to get 
this merged in soon!

> nodetool compactionstats should say which disk a compaction is writing to
> -------------------------------------------------------------------------
>                 Key: CASSANDRA-13010
>                 URL: https://issues.apache.org/jira/browse/CASSANDRA-13010
>             Project: Cassandra
>          Issue Type: New Feature
>          Components: Compaction, Tools
>            Reporter: Jon Haddad
>            Assignee: Alex Lourie
>            Priority: Major
>              Labels: lhf
>         Attachments: 13010.patch, cleanup.png, multiple operations.png

This message was sent by Atlassian JIRA

To unsubscribe, e-mail: commits-unsubscr...@cassandra.apache.org
For additional commands, e-mail: commits-h...@cassandra.apache.org

Reply via email to