Murtadha Hubail has posted comments on this change. Change subject: Enabled Feed Tests and Added External Library tests ......................................................................
Patch Set 13: (48 comments) The rest of my comments. Nothing major. There is some C++ code style, but it can be cleaned in a different change. https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedUtils.java File asterix-external-data/src/main/java/org/apache/asterix/external/util/FeedUtils.java: Line 90: if (feedLogManager.exists()) { if(!feedLogManager.exists(){ feedLogManager.create(); } feedLogManager.open(); https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/test/java/org/apache/asterix/external/classad/AMutableCharArrayString.java File asterix-external-data/src/test/java/org/apache/asterix/external/classad/AMutableCharArrayString.java: Line 37: value = new char[64]; replace 64 by increment Line 235: public int fistNonDigitChar() { first https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/test/java/org/apache/asterix/external/classad/AttributeReference.java File asterix-external-data/src/test/java/org/apache/asterix/external/classad/AttributeReference.java: Line 97: boolean success = true; success value is never changed. In case of failure, an exception will be thrown. If this isn't the required behavior, then catch the exception and return false. https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/test/java/org/apache/asterix/external/classad/BuiltinClassAdFunctions.java File asterix-external-data/src/test/java/org/apache/asterix/external/classad/BuiltinClassAdFunctions.java: Line 50: return true; what does it mean to return true here? https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/test/java/org/apache/asterix/external/classad/InputStreamLexerSource.java File asterix-external-data/src/test/java/org/apache/asterix/external/classad/InputStreamLexerSource.java: Line 109: return null; return buffer? https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/test/java/org/apache/asterix/external/classad/Operation.java File asterix-external-data/src/test/java/org/apache/asterix/external/classad/Operation.java: Line 41: /// List of supported operators WS Line 110: * WS Line 133: * WS Line 147: * WS Line 163: * WS Line 674: // if op is binary, but not associative or commutative, disallow splitting WS Line 826: // same operators on both children . since op!=NO_OP, neither are op1, WS Line 927: // comparison between strings and non-exceptional non-string WS Line 1218: // trap sigfpe and set the ClassAdExprFPE flag to true; on NT check the WS https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/test/java/org/apache/asterix/external/classad/test/FunctionalTester.java File asterix-external-data/src/test/java/org/apache/asterix/external/classad/test/FunctionalTester.java: Line 135: * WS Line 145: // Then we parse to see what the user wants. WS Line 219: * WS Line 236: * WS Line 261: * WS Line 274: // We don't increment it until we use it. WS Line 327: * WS Line 371: // We have to be careful with substr() because with gcc 2.96, it likes to WS Line 457: * WS Line 520: * WS Line 544: * WS Line 573: * WS Line 587: * WS Line 630: * WS Line 649: * WS Line 675: * WS Line 756: * WS Line 764: WS Line 791: WS Line 795: * WS Line 803: WS Line 811: WS Line 827: WS Line 984: * WS Line 1007: * WS Line 1056: * WS Line 1083: * WS Line 1100: // We have to be careful with substr() because with gcc 2.96, it likes to WS https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-external-data/src/test/java/org/apache/asterix/external/library/ClassAdParser.java File asterix-external-data/src/test/java/org/apache/asterix/external/library/ClassAdParser.java: Line 139: * WS Line 658: * WS Line 727: * WS Line 794: * WS https://asterix-gerrit.ics.uci.edu/#/c/625/13/asterix-installer/src/test/resources/integrationts/library/queries/library-parsers/record-parser/record-parser.2.update.aql File asterix-installer/src/test/resources/integrationts/library/queries/library-parsers/record-parser/record-parser.2.update.aql: Line 18: */ This file is not needed -- To view, visit https://asterix-gerrit.ics.uci.edu/625 To unsubscribe, visit https://asterix-gerrit.ics.uci.edu/settings Gerrit-MessageType: comment Gerrit-Change-Id: Idd1fccd136fa2645b2707bbf7c04e60991ae8d4a Gerrit-PatchSet: 13 Gerrit-Project: asterixdb Gerrit-Branch: master Gerrit-Owner: abdullah alamoudi <[email protected]> Gerrit-Reviewer: Ian Maxon <[email protected]> Gerrit-Reviewer: Jenkins <[email protected]> Gerrit-Reviewer: Michael Blow <[email protected]> Gerrit-Reviewer: Murtadha Hubail <[email protected]> Gerrit-Reviewer: Till Westmann <[email protected]> Gerrit-Reviewer: abdullah alamoudi <[email protected]> Gerrit-HasComments: Yes
