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.

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

Fix linting when creating a pipeline skipping some parts of the template #2330

Merged
merged 5 commits into from
Jun 21, 2023

Conversation

mirpedrol
Copy link
Member

Fix linting when createing a pipeline skipping some parts of the template

Close #2329

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

@mirpedrol mirpedrol changed the title Fix linting when createing a pipeline skipping some parts of the template Fix linting when creating a pipeline skipping some parts of the template Jun 20, 2023
@@ -446,6 +469,10 @@ def fix_linting(self):
if not self.template_params["github_badges"] or not self.template_params["github"]:
lint_config["readme"] = ["nextflow_badge"]

# If the pipeline is unbranded
if not self.template_params["branded"]:
lint_config["files_unchanged"].extend([".github/ISSUE_TEMPLATE/bug_report.yml"])
Copy link
Member

Choose a reason for hiding this comment

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

If we can avoid just flat out ignoring this file, that would be nice. There is already a function to customise it to remove some nf-core specific stuf. I think the failure on this file is something to do with the custom prefix / name match at the end of the output.

Copy link
Member Author

Choose a reason for hiding this comment

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

When the pipeline is unbranded we remove one chunk of this file containing links to nf-core documentation:

  - type: markdown
    attributes:
      value: |
        Before you post this issue, please check the documentation:

        - [nf-core website: troubleshooting](https://nf-co.re/usage/troubleshooting)
        - [{{ name }} pipeline documentation](https://nf-co.re/{{ short_name }}/usage)

so it won't match the template, and can't be considered a partial match either.

Copy link
Member

Choose a reason for hiding this comment

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

so it won't match the template, and can't be considered a partial match either.

Seems like this isn't possible so will merge this in and we can follow up in a separate PR / issue if required.

@mirpedrol mirpedrol force-pushed the fix-linting-create-template branch from fd34e7a to 56ca280 Compare June 20, 2023 12:42
@codecov
Copy link

codecov bot commented Jun 20, 2023

Codecov Report

Merging #2330 (f5c4f4e) into dev (cf15617) will increase coverage by 0.21%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev    #2330      +/-   ##
==========================================
+ Coverage   72.86%   73.07%   +0.21%     
==========================================
  Files          78       78              
  Lines        8777     8769       -8     
==========================================
+ Hits         6395     6408      +13     
+ Misses       2382     2361      -21     
Impacted Files Coverage Δ
nf_core/create.py 82.96% <100.00%> (+7.30%) ⬆️

... and 7 files with indirect coverage changes

Copy link
Member

@drpatelh drpatelh left a comment

Choose a reason for hiding this comment

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

Tested your branch and the linting is now working for me @mirpedrol . Will approve and wait for @ewels to comment on his comment.

@drpatelh drpatelh merged commit ab8ca2e into nf-core:dev Jun 21, 2023
@mirpedrol mirpedrol deleted the fix-linting-create-template branch June 21, 2023 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants