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.

Sign up for GitHub

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

Jump to bottom

Adds test data base path to classic test configs for more easily switching between test data location #2931

Merged
merged 8 commits into from
May 7, 2024
Merged

Adds test data base path to classic test configs for more easily switching between test data location #2931

merged 8 commits into from
May 7, 2024

Conversation

jfy133
Copy link
Member

@jfy133 jfy133 commented Apr 24, 2024

https://nfcore.slack.com/archives/C043UU89KKQ/p1713940102697309

PR checklist

  • This comment contains a description of changes (with reason)
  • CHANGELOG.md is updated
  • If you've fixed a bug or added code that should be tested, add tests!
  • Documentation in docs is updated

@jfy133 jfy133 changed the base branch from master to dev April 24, 2024 06:47
@github-actions GitHub Actions
Copy link
Contributor

This PR is against the master branch ❌

  • Do not close this PR
  • Click Edit and change the base to dev
  • This CI test will remain failed until you push a new commit

Hi @jfy133,

It looks like this pull-request is has been made against the jfy133/nf-core-tools 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 jfy133/nf-core-tools 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.
Note that even after this, the test will continue to show as failing until you push a new commit.

Thanks again for your contribution!

@mashehu
Copy link
Contributor

mashehu commented Apr 24, 2024

Is there documentation about this somewhere?

@maxulysse
Copy link
Member

Is there documentation about this somewhere?

I'm on it

@jfy133
Copy link
Member Author

jfy133 commented Apr 24, 2024

Is there documentation about this somewhere?

What do you mean? It's only really aimed at developers I guess? And I updated the schema...

maxulysse
maxulysse reviewed Apr 24, 2024
View reviewed changes
CHANGELOG.md Outdated Show resolved Hide resolved
maxulysse
maxulysse reviewed Apr 24, 2024
View reviewed changes
Copy link
Member

@maxulysse maxulysse left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

need to tinker the path a bit more

nf_core/pipeline-template/conf/test.config Outdated Show resolved Hide resolved
nf_core/pipeline-template/conf/test_full.config Outdated Show resolved Hide resolved
nf_core/pipeline-template/conf/test_full.config Outdated Show resolved Hide resolved
nf_core/pipeline-template/nextflow_schema.json Outdated Show resolved Hide resolved
jfy133 and others added 3 commits April 24, 2024 12:50
@jfy133 @maxulysse
Apply suggestions from code review
cb4a775 
Co-authored-by: Maxime U Garcia <maxime.garcia@seqera.io>
@jfy133
Harshil Align™️
60911ab
@jfy133
Fix JSON typo
720852e
mirpedrol
mirpedrol approved these changes May 6, 2024
View reviewed changes
Copy link
Member

@mirpedrol mirpedrol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this ready to be merged?

@maxulysse maxulysse mentioned this pull request May 6, 2024
Merged
11 tasks
mashehu and others added 2 commits May 7, 2024 15:16
@mashehu @maxulysse
Apply suggestions from code review
f08f0e4 
Co-authored-by: Maxime U Garcia <maxime.garcia@seqera.io>
@mashehu
add default value to schema
d4c8f36
@mashehu mashehu merged commit 1c2ce6c into nf-core:dev May 7, 2024
34 checks passed
@dependabot dependabot bot mentioned this pull request Jun 3, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@maxulysse maxulysse maxulysse left review comments

@mirpedrol mirpedrol mirpedrol approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jfy133 @mashehu @maxulysse @mirpedrol