Luo Chen has posted comments on this change.

Change subject: Implemented LSM disk components alignment
......................................................................


Patch Set 17:

(20 comments)

> (29 comments)
 > 
 > Added some "non-technical" comments.

https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.1.ddl.aql
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.1.ddl.aql:

PS16, Line 21:  
> White space
Done


PS16, Line 22:   
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.2.update.aql
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.2.update.aql:

PS16, Line 21: policy
> White space.
Sorry that not quite familiar with the format requirements. Now they're fixed.


PS16, Line 21:  
> Ws
Done


https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.3.update.aql
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.3.update.aql:

PS16, Line 21:  
> WS
Done


PS16, Line 22:   
> WS
Done


PS16, Line 26:   
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.4.query.aql
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.4.query.aql:

PS16, Line 21:  
> WS
Done


PS16, Line 22:   
> WS
Done


PS16, Line 27:      
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.2.update.sqlpp
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.2.update.sqlpp:

PS16, Line 21:  
> WS
Done


PS16, Line 22:   
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.3.update.sqlpp
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.3.update.sqlpp:

PS16, Line 21:  
> WS
Done


PS16, Line 22:   
> WS
Done


PS16, Line 25:  
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.4.query.sqlpp
File 
asterixdb/asterix-app/src/test/resources/runtimets/queries_sqlpp/dml/using-correlated-prefix-merge-policy-with-feed/using-correlated-prefix-merge-policy-with-feed.4.query.sqlpp:

PS16, Line 21:  
> WS
Done


PS16, Line 22:   
> WS
Done


PS16, Line 27:      
> WS
Done


https://asterix-gerrit.ics.uci.edu/#/c/1725/16/asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java
File 
asterixdb/asterix-common/src/main/java/org/apache/asterix/common/context/CorrelatedPrefixMergePolicy.java:

PS16, Line 44: Logger
> Why log4j instead java.util.logging?
Logger is not actually used. Removed this import.


PS16, Line 95: IllegalStateException
> Use error code and align with the exception guideline:
Got it. BTW, I saw "All message templates should be tested.", but where should 
we test them? (searched the codebase, but didn't find any tests for them...)


-- 
To view, visit https://asterix-gerrit.ics.uci.edu/1725
To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I64bf34e255def72adc73b9f87cfa628a172ea694
Gerrit-PatchSet: 17
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Luo Chen <[email protected]>
Gerrit-Reviewer: Ian Maxon <[email protected]>
Gerrit-Reviewer: Jenkins <[email protected]>
Gerrit-Reviewer: Jianfeng Jia <[email protected]>
Gerrit-Reviewer: Luo Chen <[email protected]>
Gerrit-Reviewer: Till Westmann <[email protected]>
Gerrit-Reviewer: Yingyi Bu <[email protected]>
Gerrit-Reviewer: abdullah alamoudi <[email protected]>
Gerrit-HasComments: Yes

Reply via email to