Patch 2.2.1 by jfy133 · Pull Request #594 · nf-core/eager

jfy133 · 2020-10-27T08:26:30Z

nf-core/eager pull request

This is a draft PR for first 2.2 patch, prior version bumping.

Once approved, will bump versions to 2.2.1 and make proper PR.

To close #591 and close #592 and close #590 and close #596 and to close #582

PR checklist

This comment contains a description of changes (with reason)
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/eager branch on the nf-core/test-datasets repo
Ensure the test suite passes (nextflow run . -profile test,docker --paired_end).
Make sure your code lints (nf-core lint .).
Documentation in docs is updated
CHANGELOG.md is updated
README.md is updated

Learn more about contributing: CONTRIBUTING.md

Put 2.2.1 in better named branch

github-actions · 2020-10-27T08:26:43Z

Hi @jfy133,

It looks like this pull-request is has been made against the nf-core/eager master branch.
The master branch on nf-core repositories should always contain code from the latest release.
Because of this, PRs to master are only allowed if they come from the nf-core/eager dev branch.

You do not need to close this PR, you can change the target branch to dev by clicking the "Edit" button at the top of this page.

Thanks again for your contribution!

github-actions · 2020-10-27T12:12:16Z

`nf-core lint` overall result: Failed ❌

Updated for pipeline commit 4693b8b

+| ✅ 67 tests passed       |+
!| ❗  1 tests had warnings |!
-| ❌  1 tests failed       |-

Details

❌ Test failures:

Test #5 - GitHub Actions AWS test should be triggered on workflow_dispatch and not on push or PRs: /home/runner/work/eager/eager/.github/workflows/awstest.yml

❗ Test warnings:

Test #4 - Config manifest.version should end in dev: '2.2.1'

✅ Tests passed:

Test #1 - File found: nextflow.config
Test #1 - File found: nextflow_schema.json
Test #1 - File found: LICENSE or LICENSE.md or LICENCE or LICENCE.md
Test #1 - File found: README.md
Test #1 - File found: CHANGELOG.md
Test #1 - File found: docs/README.md
Test #1 - File found: docs/output.md
Test #1 - File found: docs/usage.md
Test #1 - File found: .github/workflows/branch.yml
Test #1 - File found: .github/workflows/ci.yml
Test #1 - File found: .github/workflows/linting.yml
Test #1 - File found: main.nf
Test #1 - File found: environment.yml
Test #1 - File found: Dockerfile
Test #1 - File found: conf/base.config
Test #1 - File found: .github/workflows/awstest.yml
Test #1 - File found: .github/workflows/awsfulltest.yml
Test #1 - File not found check: Singularity
Test #1 - File not found check: parameters.settings.json
Test #1 - File not found check: bin/markdown_to_html.r
Test #1 - File not found check: .travis.yml
Test #3 - Licence check passed
Test #2 - Dockerfile check passed
Test #4 - Config variable found: manifest.name
Test #4 - Config variable found: manifest.nextflowVersion
Test #4 - Config variable found: manifest.description
Test #4 - Config variable found: manifest.version
Test #4 - Config variable found: manifest.homePage
Test #4 - Config variable found: timeline.enabled
Test #4 - Config variable found: trace.enabled
Test #4 - Config variable found: report.enabled
Test #4 - Config variable found: dag.enabled
Test #4 - Config variable found: process.cpus
Test #4 - Config variable found: process.memory
Test #4 - Config variable found: process.time
Test #4 - Config variable found: params.outdir
Test #4 - Config variable found: params.input
Test #4 - Config variable found: manifest.mainScript
Test #4 - Config variable found: timeline.file
Test #4 - Config variable found: trace.file
Test #4 - Config variable found: report.file
Test #4 - Config variable found: dag.file
Test #4 - Config variable found: process.container
Test #4 - Config variable (correctly) not found: params.version
Test #4 - Config variable (correctly) not found: params.nf_required_version
Test #4 - Config variable (correctly) not found: params.container
Test #4 - Config variable (correctly) not found: params.singleEnd
Test #4 - Config variable (correctly) not found: params.igenomesIgnore
Test #4 - Config timeline.enabled had correct value: true
Test #4 - Config report.enabled had correct value: true
Test #4 - Config trace.enabled had correct value: true
Test #4 - Config dag.enabled had correct value: true
Test #4 - Config manifest.name began with nf-core/
Test #4 - Config variable manifest.homePage began with https://github.com/nf-core/
Test #4 - Config dag.file ended with .svg
Test #4 - Config variable manifest.nextflowVersion started with >= or !>=
Test #4 - Config process.container looks correct: nfcore/eager:2.2.1
Test #5 - GitHub Actions 'branch' workflow is triggered for PRs to master: /home/runner/work/eager/eager/.github/workflows/branch.yml
Test #5 - GitHub Actions 'branch' workflow looks good: /home/runner/work/eager/eager/.github/workflows/branch.yml
Test #5 - GitHub Actions CI is triggered on expected events: /home/runner/work/eager/eager/.github/workflows/ci.yml
Test #5 - CI is building the correct docker image: docker build --no-cache . -t nfcore/eager:2.2.1
Test #5 - CI is pulling the correct docker image: docker pull nfcore/eager:dev
Test #5 - CI is tagging docker image correctly: docker tag nfcore/eager:dev nfcore/eager:2.2.1
Test #5 - Continuous integration checks minimum NF version: /home/runner/work/eager/eager/.github/workflows/ci.yml
Test #5 - GitHub Actions linting workflow is triggered on PR and push: /home/runner/work/eager/eager/.github/workflows/linting.yml
Test #5 - Continuous integration runs Markdown lint Tests: /home/runner/work/eager/eager/.github/workflows/linting.yml
Test #5 - Continuous integration runs nf-core lint Tests: /home/runner/work/eager/eager/.github/workflows/linting.yml

Run details:

nf-core/tools version 1.11
Run at 2020-10-27 12:25:14

mashehu

While doing other things, I saw two smaller things in the docs you should change as well.

in usage.md please add a # Introduction after the Table of contents, so that it will not be rendered on the website.
in 'nextflow_schema.json`, could you please change fa-forward icons (:fast_forward:) to fa-fast-forward, for consistency with the other pipelines

mashehu · 2020-10-29T15:55:27Z

+> Note that with this you will _not_ have the automatic retry mechanism. If
+> you want this, re-add the `check_max()` function on the `memory` line, and
+> add to the bottom of the entire file (outside the profiles block), the
+> block starting `def check_max(obj, type) {`, which is at the end of the
+> [nextflow.config file](https://github.com/nf-core/eager/blob/master/nextflow.config)
+


You could point to the old version of the file, to show that more clearly.

What do you mean old version?

To point to a version where you have this still in or to something similar.

Ah, I see - good point. I don't think it'll be removed for a long time as it's in the nf-core template and AFAIK it would still be valid for DSL2. But I'll note that for the next patch (had to release today because I'm running a workshop tomorrow and needed to make sure it's working 😅 )

jfy133 added 5 commits October 23, 2020 14:54

Fix image issue in #591

31a47bc

Fix Issue #592

6e57f33

Merge pull request #593 from nf-core/2.2.1

46e6274

Put 2.2.1 in better named branch

Fix CHANGELOG linting

de6e4e0

Remove dockstore

613ebd6

jfy133 marked this pull request as draft October 27, 2020 08:26

jfy133 requested a review from apeltzer October 27, 2020 08:27

Address #596

aa643e3

jfy133 added 4 commits October 27, 2020 13:15

Version bump for patch

2ccbef9

Bump nf-core tools version

4693b8b

Fix nf-core 1.11 issue

7c28301

CHANGELOG

36ca27a

This was referenced Oct 27, 2020

Remove dockstore from root #590

Closed

Merge in template 1.11 once its there #589

Closed

jfy133 added 4 commits October 27, 2020 14:15

Address #582

d618e38

CHANGELOG

5b942dd

CHANGELOG

74e9255

Linting

fd58887

jfy133 marked this pull request as ready for review October 28, 2020 08:32

apeltzer approved these changes Oct 28, 2020

View reviewed changes

mashehu approved these changes Oct 29, 2020

View reviewed changes

jfy133 added 2 commits October 29, 2020 17:35

Changes from code review from @mashehu

6838a14

Missing header level

8ad3437

jfy133 merged commit 7971d89 into master Oct 29, 2020

jfy133 deleted the patch-2.2.1 branch October 29, 2020 18:05

jfy133 restored the patch-2.2.1 branch October 29, 2020 18:05

jfy133 deleted the patch-2.2.1 branch November 3, 2020 11:28

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Patch 2.2.1#594

Patch 2.2.1#594
jfy133 merged 16 commits intomasterfrom
patch-2.2.1

jfy133 commented Oct 27, 2020 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 27, 2020

Uh oh!

github-actions Bot commented Oct 27, 2020 •

edited

Loading

❌ Test failures:

❗ Test warnings:

✅ Tests passed:

Run details:

Uh oh!

mashehu left a comment

Uh oh!

mashehu Oct 29, 2020

Uh oh!

jfy133 Oct 29, 2020

Uh oh!

mashehu Oct 29, 2020

Uh oh!

jfy133 Oct 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

jfy133 commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

nf-core/eager pull request

PR checklist

Uh oh!

github-actions Bot commented Oct 27, 2020

Uh oh!

github-actions Bot commented Oct 27, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

nf-core lint overall result: Failed ❌

❌ Test failures:

❗ Test warnings:

✅ Tests passed:

Run details:

Uh oh!

mashehu left a comment

Choose a reason for hiding this comment

Uh oh!

mashehu Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

jfy133 Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

mashehu Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

jfy133 Oct 29, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jfy133 commented Oct 27, 2020 •

edited

Loading

github-actions Bot commented Oct 27, 2020 •

edited

Loading

`nf-core lint` overall result: Failed ❌