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

random nits #437

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@ryan-williams
Member

ryan-williams commented Oct 27, 2014

  • disable scalariform's auto-rewriting of dangling parends. lmk if you guys feel strongly that this should never be done (vs. were just inheriting a scalariform default), I like the option to use a dangling close-parend in some cases.
  • wrap some long lines and update some comments
@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Oct 27, 2014

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

Build result: FAILURE

GitHub pull request #437 of commit 28b528e automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/437/merge^{commit} # timeout=10Checking out Revision 620ad77 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 620ad77 > git rev-list 77ea367 # timeout=10Triggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosTriggering ADAM-prb » 2.2.0,centosADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILUREADAM-prb » 2.2.0,centos completed with result FAILURE
Test FAILed.

AmplabJenkins commented Oct 27, 2014

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

Build result: FAILURE

GitHub pull request #437 of commit 28b528e automatically merged.[EnvInject] - Loading node environment variables.Building remotely on amp-jenkins-slave-01 (centos) in workspace /home/jenkins/workspace/ADAM-prb > git rev-parse --is-inside-work-tree # timeout=10Fetching changes from the remote Git repository > git config remote.origin.url https://github.com/bigdatagenomics/adam.git # timeout=10Fetching upstream changes from https://github.com/bigdatagenomics/adam.git > git --version # timeout=10 > git fetch --tags --progress https://github.com/bigdatagenomics/adam.git +refs/pull/:refs/remotes/origin/pr/ > git rev-parse origin/pr/437/merge^{commit} # timeout=10Checking out Revision 620ad77 (detached) > git config core.sparsecheckout # timeout=10 > git checkout -f 620ad77 > git rev-list 77ea367 # timeout=10Triggering ADAM-prb » 1.0.4,centosTriggering ADAM-prb » 2.3.0,centosTriggering ADAM-prb » 2.2.0,centosADAM-prb » 1.0.4,centos completed with result FAILUREADAM-prb » 2.3.0,centos completed with result FAILUREADAM-prb » 2.2.0,centos completed with result FAILURE
Test FAILed.

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Oct 27, 2014

Member

hm I'm guessing there is something wrong with @AmplabJenkins right now? mvn test is passing for me locally both here and for #438, and this failure appears on both

Member

ryan-williams commented Oct 27, 2014

hm I'm guessing there is something wrong with @AmplabJenkins right now? mvn test is passing for me locally both here and for #438, and this failure appears on both

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Oct 27, 2014

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

AmplabJenkins commented Oct 27, 2014

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

@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Oct 27, 2014

Member

+1, LGTM. Can you squash this down?

Member

fnothaft commented Oct 27, 2014

+1, LGTM. Can you squash this down?

@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Oct 27, 2014

Member

cool, squashed, @fnothaft!

Member

ryan-williams commented Oct 27, 2014

cool, squashed, @fnothaft!

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Oct 27, 2014

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

AmplabJenkins commented Oct 27, 2014

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

@tdanford

This comment has been minimized.

Show comment
Hide comment
@tdanford

tdanford Oct 29, 2014

Contributor

@ryan-williams if you rebase this, I will merge it.

Contributor

tdanford commented Oct 29, 2014

@ryan-williams if you rebase this, I will merge it.

random nits and cleanups:
- make scalariform allow dangling parends

  sometimes I find this to be a useful way to indicate nesting:

  throw new Exception(
      “%s %s %s %s”.format(
          foo, bar baz, boo
      )
  )

- tweak comment about read-ID suffixes in FASTQ

- wrap a few long lines

- add docs to adamFastqLoad method
@ryan-williams

This comment has been minimized.

Show comment
Hide comment
@ryan-williams

ryan-williams Oct 30, 2014

Member

thanks @tdanford, it is rebased now!

Member

ryan-williams commented Oct 30, 2014

thanks @tdanford, it is rebased now!

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Oct 30, 2014

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

AmplabJenkins commented Oct 30, 2014

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

@AmplabJenkins

This comment has been minimized.

Show comment
Hide comment
@AmplabJenkins

AmplabJenkins Oct 30, 2014

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

AmplabJenkins commented Oct 30, 2014

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

fnothaft added a commit that referenced this pull request Oct 30, 2014

Merge pull request #437 from ryan-williams/nits
[ADAM-437] random nits and cleanups:
@fnothaft

This comment has been minimized.

Show comment
Hide comment
@fnothaft

fnothaft Oct 30, 2014

Member

Thanks @ryan-williams! Merged manually via b33f54e.

Member

fnothaft commented Oct 30, 2014

Thanks @ryan-williams! Merged manually via b33f54e.

@fnothaft fnothaft closed this Oct 30, 2014

This was referenced Oct 31, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment