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

115 add flye to nf corebacass #116

Closed
wants to merge 7 commits into from
Closed

115 add flye to nf corebacass #116

wants to merge 7 commits into from

Conversation

lborcard
Copy link

@lborcard lborcard commented Mar 18, 2024 •
edited
Loading

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 you've added a new tool - have you followed the pipeline conventions in the contribution docs
  • If necessary, also make a PR on the nf-core/bacass branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Usage Documentation in docs/usage.md is updated.
  • Output Documentation in docs/output.md is updated.
  • CHANGELOG.md is updated.
  • README.md is updated (including new tool citations and authors/contributors).

@github-actions GitHub Actions
Copy link

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 @lborcard,

It looks like this pull-request is has been made against the nf-core/bacass 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/bacass 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!

@github-actions GitHub Actions
Copy link

Python linting (black) is failing

To keep the code consistent with lots of contributors, we run automated code consistency checks.
To fix this CI test, please run:

  • Install black: pip install black
  • Fix formatting errors in your pipeline: black .

Once you push these changes the test should pass, and you can hide this comment 👍

We highly recommend setting up Black in your code editor so that this formatting is done automatically on save. Ask about it on Slack for help!

Thanks again for your contribution!

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 18, 2024 •
edited
Loading

nf-core lint overall result: Failed ❌

Posted for pipeline commit 4bfb20d

+| ✅ 159 tests passed       |+
#| ❔   1 tests were ignored |#
!| ❗   3 tests had warnings |!
-| ❌  17 tests failed       |-

❌ Test failures:

  • files_exist - File must be removed: lib/Utils.groovy
  • files_exist - File must be removed: lib/WorkflowMain.groovy
  • files_exist - File must be removed: lib/NfcoreTemplate.groovy
  • files_exist - File must be removed: lib/WorkflowBacass.groovy
  • files_exist - File must be removed: lib/nfcore_external_java_deps.jar
  • nextflow_config - Config default value incorrect: params.dfast_config is set as /storage/tmp/scratch_lb21z855/nf-core-hackathon24/bacass/assets/test_config_dfast.py in nextflow_schema.json but is /home/runner/work/bacass/bacass/assets/test_config_dfast.py in nextflow.config.
  • files_unchanged - .github/CONTRIBUTING.md does not match the template
  • files_unchanged - .github/PULL_REQUEST_TEMPLATE.md does not match the template
  • files_unchanged - .github/workflows/branch.yml does not match the template
  • files_unchanged - .github/workflows/linting_comment.yml does not match the template
  • files_unchanged - .github/workflows/linting.yml does not match the template
  • files_unchanged - assets/email_template.html does not match the template
  • files_unchanged - assets/email_template.txt does not match the template
  • files_unchanged - assets/nf-core-bacass_logo_light.png does not match the template
  • files_unchanged - docs/images/nf-core-bacass_logo_light.png does not match the template
  • files_unchanged - docs/images/nf-core-bacass_logo_dark.png does not match the template
  • files_unchanged - pyproject.toml does not match the template

❗ Test warnings:

❔ Tests ignored:

✅ Tests passed:

Run details

  • nf-core/tools version 2.13.1
  • Run at 2024-03-18 16:12:56

@lborcard lborcard changed the base branch from master to dev March 18, 2024 16:16
@lborcard lborcard requested a review from d4straub March 18, 2024 16:26
maxulysse
maxulysse reviewed Mar 18, 2024
View reviewed changes
nextflow_schema.json Outdated Show resolved Hide resolved
maxulysse
maxulysse reviewed Mar 18, 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.

Can you update CHANGELOG as well?

@Daniel-VM
Copy link
Contributor

The " does not match the template" errors are on my side, I believe. I still have to make the PR with the nf-core 2.13.1 template update.

lborcard and others added 2 commits March 18, 2024 17:38
@lborcard @maxulysse
Update nextflow_schema.json
391f8a1 
Co-authored-by: Maxime U Garcia <max.u.garcia@gmail.com>
@lborcard
added change log
97240bc
@lborcard
Copy link
Author

Can you update CHANGELOG as well?

ok I added this pull request to the changelog

@lborcard lborcard requested a review from maxulysse March 18, 2024 17:10
lborcard
lborcard commented Mar 19, 2024
View reviewed changes
Copy link
Author

@lborcard lborcard left a comment

Choose a reason for hiding this comment

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

nothing to report

@lborcard
Merge branch '115-add-flye-to-nf-corebacass' of github.com:nf-core/ba…
d0c46ee 
…cass into 115-add-flye-to-nf-corebacass

merging the template changes.
@lborcard
Copy link
Author

I dont understand what happened to the template?

@Daniel-VM
Copy link
Contributor

I need to first make a PR and merge into dev the nf-core tools template v2.13.1 (hopefully in a couple of hours). Since current version of nf-core/bacass is bit older, the git CI, which relies on latest nf-core tools is rasing errors due to missing or unexpected files/file-contents.

@lborcard
Copy link
Author

I need to first make a PR and merge into dev the nf-core tools template v2.13.1 (hopefully in a couple of hours). Since current version of nf-core/bacass is bit older, the git CI, which relies on latest nf-core tools is rasing errors due to missing or unexpected files/file-contents.

Ok so what should I do in the meantime?

@Daniel-VM
Copy link
Contributor

Daniel-VM commented Mar 19, 2024 •
edited
Loading

I was reviewing your changes and I found that most dev implementations such as dragonflye disappeared 🤔. Wait, and also changelog addition should be added to section : v2.2.0dev nf-core/bacass: "Aqua Platinum Zebrafish". Bit it doesn't exist....

@lborcard, this in comming branch is not from your forked but from another one directly created in the original nf-core/bacass repo.

When attepting to make PR you sould do it form your forked_repo:branch_name to nf-core_original_repo:dev. See this one: #117 .

(@d4straub ⚠️ ).

@Daniel-VM
Copy link
Contributor

@lborcard here are nf-core contribiting guidelines that might help you

@lborcard
Copy link
Author

Sorry about that it kind of Naturally created this new branch when I used the nf-core tools and I didn't see this section of the instructions. My mistake I will fork it and merge my changes and start the process again. My apologies.

@Daniel-VM
Copy link
Contributor

Daniel-VM commented Mar 19, 2024 •
edited
Loading

No worries. I am closing this PR now. Remember: when working locally, checkout first to 'dev' branch, then create a new branch. This way, your new branch will have the latest development code, reducing the probability of conflicts when making a PR against nf-core:dev.

@Daniel-VM Daniel-VM closed this Mar 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@d4straub d4straub Awaiting requested review from d4straub

@maxulysse maxulysse Awaiting requested review from maxulysse

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

Successfully merging this pull request may close these issues.
5 participants
@lborcard @Daniel-VM @maxulysse @d4straub @mashehu