Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add support for Array-Type in JSON #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

muditsin
Copy link
Contributor

Fixes: #35

@AmplabJenkins
Copy link

Can one of the admins verify this patch?

@muditsin
Copy link
Contributor Author

@anuragkh PR is currently WIP, but wanted to check if the approach was alright

@anuragkh
Copy link
Collaborator

Hi @muditsin, thanks for contributing!

The approach looks about right to me, although there is some code duplication (e.g., parsing the values in the array is the same as parsing individual values). Perhaps making them modular might help reduce code redundancy.

@muditsin
Copy link
Contributor Author

muditsin commented Feb 2, 2019

@anuragkh Updated to reduce duplication. Thanks!

@anuragkh
Copy link
Collaborator

anuragkh commented Feb 2, 2019

Jenkins, test this please.

1 similar comment
@anuragkh
Copy link
Collaborator

anuragkh commented Feb 2, 2019

Jenkins, test this please.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Succinct-PRB/3/

Build result: FAILURE

[...truncated 216 lines...][INFO] ------------------------------------------------------------------------Waiting for Jenkins to finish collecting data[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:2.5.1:compile (default-compile) on project succinct-serde: Compilation failure[ERROR] /home/jenkins/workspace/Succinct-PRB/serde/src/main/java/edu/berkeley/cs/succinct/object/deserializer/JsonDeserializer.java:[119,54] error: ';' expected[ERROR] -> [Help 1][ERROR] [ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.[ERROR] Re-run Maven using the -X switch to enable full debug logging.[ERROR] [ERROR] For more information about the errors and possible solutions, please read the following articles:[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException[ERROR] [ERROR] After correcting the problems, you can resume the build with the command[ERROR] mvn -rf :succinct-serde[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/assembly/pom.xml to amplab/succinct-assembly/0.1.8/succinct-assembly-0.1.8.pom[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/pom.xml to amplab/succinct/0.1.8/succinct-0.1.8.pom[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/spark/pom.xml to amplab/succinct-spark/0.1.8/succinct-spark-0.1.8.pom[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/serde/pom.xml to amplab/succinct-serde/0.1.8/succinct-serde-0.1.8.pom[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/core/pom.xml to amplab/succinct-core/0.1.8/succinct-core-0.1.8.pom[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/core/target/succinct-core-0.1.8.jar to amplab/succinct-core/0.1.8/succinct-core-0.1.8.jar[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/core/target/succinct-core-0.1.8-tests.jar to amplab/succinct-core/0.1.8/succinct-core-0.1.8-tests.jar[JENKINS] Archiving /home/jenkins/workspace/Succinct-PRB/core/target/succinct-core-0.1.8-jar-with-dependencies.jar to amplab/succinct-core/0.1.8/succinct-core-0.1.8-jar-with-dependencies.jarSending e-mails to: anuragk@berkeley.educhannel stopped
Test FAILed.

@muditsin
Copy link
Contributor Author

muditsin commented Feb 3, 2019

Jenkins, test this please.

2 similar comments
@muditsin
Copy link
Contributor Author

muditsin commented Feb 6, 2019

Jenkins, test this please.

@anuragkh
Copy link
Collaborator

anuragkh commented Feb 6, 2019

Jenkins, test this please.

@anuragkh
Copy link
Collaborator

anuragkh commented Feb 6, 2019

@muditsin, Jenkins is only triggered by repo admins and whitelisted committers atm. I'll add you to whitelist.

@anuragkh
Copy link
Collaborator

anuragkh commented Feb 6, 2019

Jenkins, add to whitelist.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Succinct-PRB/4/
Test PASSed.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/Succinct-PRB/5/
Test PASSed.

@anuragkh
Copy link
Collaborator

anuragkh commented Feb 6, 2019

@muditsin Can you please add tests for the new data types you have added?

@muditsin
Copy link
Contributor Author

muditsin commented Feb 6, 2019

@anuragkh Sure

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants