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 multiqc_config.yml links #2372

Merged
merged 9 commits into from
Jul 18, 2023
Merged

Conversation

mirpedrol
Copy link
Member

Close #2361
Needs the changes in nf-core/website#1859

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

@mashehu
Copy link
Contributor

mashehu commented Jul 13, 2023

I would prefer to keep the version and for X.X.Xdev to link to simply link to dev. Or would that not work with the linting?

@mirpedrol
Copy link
Member Author

I think it will work, I will modify it :)

@codecov
Copy link

codecov bot commented Jul 13, 2023

Codecov Report

Merging #2372 (19a19f6) into dev (a353abb) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##              dev    #2372   +/-   ##
=======================================
  Coverage   72.80%   72.81%           
=======================================
  Files          78       78           
  Lines        8888     8891    +3     
=======================================
+ Hits         6471     6474    +3     
  Misses       2417     2417           
Impacted Files Coverage Δ
nf_core/bump_version.py 91.52% <100.00%> (+0.14%) ⬆️
nf_core/lint/multiqc_config.py 67.79% <100.00%> (+0.55%) ⬆️

... and 1 file with indirect coverage changes

@mirpedrol mirpedrol changed the title Remove pipeline version from multiqc_config.yml links Fix multiqc_config.yml links Jul 13, 2023
@mirpedrol mirpedrol requested a review from mashehu July 18, 2023 07:28
@@ -75,9 +75,9 @@ def multiqc_config(self):
if "report_comment" not in mqc_yml:
raise AssertionError()
if mqc_yml["report_comment"].strip() != (
f'This report has been generated by the <a href="https://github.com/nf-core/{self.pipeline_name}/{version}" '
f'This report has been generated by the <a href="https://github.com/nf-core/{self.pipeline_name}/tree/{version}" '
Copy link
Contributor

@SusiJo SusiJo Jul 18, 2023

Choose a reason for hiding this comment

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

Comment on line 74:
What about checking if the 'manifest.version' contains the 'dev' tag? Then it would also be possible to refer to the dev version of the pipeline and documentation and wouldn't create a false link.

if ("dev" in version):
    version = "dev"

Copy link
Member Author

Choose a reason for hiding this comment

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

good idea, thanks for the suggestion! I have pushed the changes :)

@mirpedrol mirpedrol merged commit 5e5b41a into nf-core:dev Jul 18, 2023
20 checks passed
@mirpedrol mirpedrol deleted the multiqc-cofig-links branch July 18, 2023 10:29
@mirpedrol mirpedrol restored the multiqc-cofig-links branch November 3, 2023 13:08
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