lncpipe icon indicating copy to clipboard operation
lncpipe copied to clipboard

Pseudo-PR: First code review

Open likelet opened this issue 7 years ago • 4 comments

Many thanks to contributing to nf-core/lncpipe!

Please fill in the appropriate checklist below (delete whatever is not relevant). These are the most common things requested on pull requests (PRs).

PR checklist

  • [x] This comment contains a description of changes (with reason)
  • [x] If you've fixed a bug or added code that should be tested, add tests!
  • [ ] If necessary, also make a PR on the nf-core/lncpipe branch on the nf-core/test-datasets repo
  • [x] Ensure the test suite passes (nextflow run . -profile test,docker).
  • [x] Make sure your code lints (nf-core lint .).
  • [x] Documentation in docs is updated
  • [ ] CHANGELOG.md is updated
  • [x] README.md is updated

Learn more about contributing: https://github.com/nf-core/lncpipe/tree/master/.github/CONTRIBUTING.md

likelet avatar Nov 23 '18 07:11 likelet

BTW, I didn't add test data here, as the test data is extremely big (about 2 Gb). I created a test data repo on testdata and stored the data at a local server that allowing remote access. Plz let me know if there is an alternative strategy.

likelet avatar Nov 23 '18 07:11 likelet

BTW, I didn't add test data here, as the test data is extremely big (about 2 Gb). I created a test data repo on testdata and stored the data at a local server that allowing remote access. Plz let me know if there is an alternative strategy.

Hosting somewhere else is fine I guess, but please still add a separate branch for lncpipe and state that in a README in that branch so that people know where the test data resides.

Do you have an idea how long your tests are running? Or could you strip the dataset down in size while still running proper tests?

apeltzer avatar Nov 23 '18 14:11 apeltzer

BTW, I didn't add test data here, as the test data is extremely big (about 2 Gb). I created a test data repo on testdata and stored the data at a local server that allowing remote access. Plz let me know if there is an alternative strategy.

Hosting somewhere else is fine I guess, but please still add a separate branch for lncpipe and state that in a README in that branch so that people know where the test data resides.

Do you have an idea how long your tests are running? Or could you strip the dataset down in size while still running proper tests?

Okey, i wll add a branch there. For now , the test data may take less than 10 mins with default setting . I tried stripping the dataset into chrX only for it can produce meanful result which also can promote the understanding to the result.

likelet avatar Nov 23 '18 14:11 likelet

BTW, I didn't add test data here, as the test data is extremely big (about 2 Gb). I created a test data repo on testdata and stored the data at a local server that allowing remote access. Plz let me know if there is an alternative strategy.

Hosting somewhere else is fine I guess, but please still add a separate branch for lncpipe and state that in a README in that branch so that people know where the test data resides.

Do you have an idea how long your tests are running? Or could you strip the dataset down in size while still running proper tests?

The test dataset has been trimmed into 0.5Gb in total, and hosted on my own server. I also set the url in the .travis.yml , and make it pass the CI building. User can also down the data with wget command. I will write a detailed document it on this.

likelet avatar Dec 06 '18 01:12 likelet