asterixdb-notifications mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Till Westmann (Code Review)" <do-not-re...@asterixdb.incubator.apache.org>
Subject Change in asterixdb[master]: ASTERIXDB-976: CSV support for all basic types
Date Tue, 13 Oct 2015 16:49:40 GMT
Till Westmann has posted comments on this change.

Change subject: ASTERIXDB-976: CSV support for all basic types
......................................................................


Patch Set 2: Code-Review+1

(7 comments)

Generally looks good to me. One thing that I noted is that there are a lot of magic constants
in the code. But I realize that fixing that might be a bigger endeavor and that we might not
want to block this change on this.

https://asterix-gerrit.ics.uci.edu/#/c/444/2/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/ACirclePrinter.java
File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/ACirclePrinter.java:

Line 44:         ps.print(" ]\"");
Do 1, 9, and 17 have a meaning that we can express in a constant name?
Or is a constant already available somewhere?


https://asterix-gerrit.ics.uci.edu/#/c/444/2/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/ALinePrinter.java
File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/ALinePrinter.java:

Line 46:         ps.print("] ]\"");
Are there constants/could we create them for 1, 9, 17, 25?


https://asterix-gerrit.ics.uci.edu/#/c/444/2/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/APoint3DPrinter.java
File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/APoint3DPrinter.java:

Line 44:         ps.print("]\"");
Same question here.


https://asterix-gerrit.ics.uci.edu/#/c/444/2/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/APointPrinter.java
File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/APointPrinter.java:

Line 42:         ps.print("]\"");
And here.


https://asterix-gerrit.ics.uci.edu/#/c/444/2/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/APolygonPrinter.java
File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/APolygonPrinter.java:

Line 57:         ps.print(" ]\"");
And here.


https://asterix-gerrit.ics.uci.edu/#/c/444/2/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/ARectanglePrinter.java
File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/ARectanglePrinter.java:

Line 47:         ps.print("] ]\"");
And here.


https://asterix-gerrit.ics.uci.edu/#/c/444/2/asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/AUUIDPrinter.java
File asterix-om/src/main/java/org/apache/asterix/dataflow/data/nontagged/printers/csv/AUUIDPrinter.java:

Line 42:         long lsb = LongPointable.getLong(b, s + 9);
And here.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I7a515efd2bbf25895537413b45eb0992484c7412
Gerrit-PatchSet: 2
Gerrit-Project: asterixdb
Gerrit-Branch: master
Gerrit-Owner: Chris Hillery <ceej@lambda.nu>
Gerrit-Reviewer: Chris Hillery <ceej@lambda.nu>
Gerrit-Reviewer: Ian Maxon <imaxon@apache.org>
Gerrit-Reviewer: Jenkins <jenkins@fulliautomatix.ics.uci.edu>
Gerrit-Reviewer: Till Westmann <tillw@apache.org>
Gerrit-HasComments: Yes

Mime
View raw message