Michael Brown has posted comments on this change.

Change subject: IMPALA-4001: qgen: add proof of concept tests for Query() 
objects
......................................................................


Patch Set 1:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/4081/1/tests/comparison/tests/fake_query.py
File tests/comparison/tests/fake_query.py:

PS1, Line 97: must at least a
> "must at least *contain* a" ...?
Done


PS1, Line 97: fram
> s/fram/from/
Done


http://gerrit.cloudera.org:8080/#/c/4081/1/tests/comparison/tests/query_object_testdata.py
File tests/comparison/tests/query_object_testdata.py:

PS1, Line 64: DATASET
> So -- these are really test cases, yes? Could this just be called QUERY_TES
Done


http://gerrit.cloudera.org:8080/#/c/4081/1/tests/comparison/tests/test_query_objects.py
File tests/comparison/tests/test_query_objects.py:

PS1, Line 25: query_something
> I'm a little confused by this. query_something is a ... QueryTest object (n
Done


Line 53: @pytest.mark.parametrize('query_test', DATASET, ids=_idfn)
> How do you feel about moving all of the tests to the end of the file, rathe
Done


-- 
To view, visit http://gerrit.cloudera.org:8080/4081
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2ed1960430ae0af469986e33f88aecb6fa74e999
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Brown <[email protected]>
Gerrit-Reviewer: David Knupp <[email protected]>
Gerrit-Reviewer: Michael Brown <[email protected]>
Gerrit-Reviewer: Taras Bobrovytsky <[email protected]>
Gerrit-HasComments: Yes

Reply via email to