Skip to content

Fix languageserver errors#726

Merged
dialvarezs merged 18 commits intonf-core:devfrom
dialvarezs:language-server-fixes
Mar 13, 2025
Merged

Fix languageserver errors#726
dialvarezs merged 18 commits intonf-core:devfrom
dialvarezs:language-server-fixes

Conversation

@dialvarezs
Copy link
Copy Markdown
Member

@dialvarezs dialvarezs commented Dec 9, 2024 •
edited
Loading

This PR solves errors/warings reported by the Nextflow language server.
Mostly reused variable names and unused variables.
Additionally, I applied module updates that include error fixes.

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/mag branch on the nf-core/test-datasets repository.
  • Make sure your code lints (nf-core pipelines lint).
  • Ensure the test suite passes (nextflow run . -profile test,docker --outdir <OUTDIR>).
  • Check for unexpected warnings in debug mode (nextflow run . -profile debug,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).

@dialvarezs dialvarezs marked this pull request as draft December 9, 2024 06:08
@dialvarezs dialvarezs changed the base branch from master to dev December 9, 2024 06:09
@nf-core nf-core deleted a comment from github-actions Bot Dec 9, 2024
@dialvarezs dialvarezs changed the title Solve languageserver errors Fix languageserver errors Dec 9, 2024
prototaxites
prototaxites reviewed Dec 18, 2024
View reviewed changes
Comment thread subworkflows/local/binning.nf Outdated
@prototaxites
Copy link
Copy Markdown
Contributor

prototaxites commented Feb 4, 2025

Two PRs that fix language server errors in modules:

nf-core/modules#7432
nf-core/modules#7431

@dialvarezs dialvarezs marked this pull request as ready for review February 16, 2025 19:52
@dialvarezs
Copy link
Copy Markdown
Member Author

dialvarezs commented Feb 16, 2025

Should I also format the local modules and subworflows with the LSP formatter as part of this PR? What do you think about that @jfy133?

@prototaxites
Copy link
Copy Markdown
Contributor

prototaxites commented Feb 17, 2025

Should I also format the local modules and subworflows with the LSP formatter as part of this PR? What do you think about that @jfy133?

IMO unless it's something we can enforce (e.g. through a linting check) it's probably not super important as future PRs might change this?

@dialvarezs
Copy link
Copy Markdown
Member Author

dialvarezs commented Feb 18, 2025

Should I also format the local modules and subworflows with the LSP formatter as part of this PR? What do you think about that @jfy133?

IMO unless it's something we can enforce (e.g. through a linting check) it's probably not super important as future PRs might change this?

Sure, you're right on that, using a linting check would be better to keep a consistent style over time.

jfy133
jfy133 approved these changes Mar 13, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@jfy133 jfy133 left a comment

Choose a reason for hiding this comment

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

Couple of comments but I think the purpose of this PR is satisfied :) thank you very much for this @dialvarezs !

Comment thread CHANGELOG.md Outdated
Comment thread modules/local/busco.nf Outdated
Comment thread subworkflows/local/gtdbtk.nf
Comment thread workflows/mag.nf Outdated
dialvarezs and others added 3 commits March 13, 2025 10:38
@dialvarezs @jfy133
Update CHANGELOG.md
b993cd1 
Co-authored-by: James A. Fellows Yates <[email protected]>
@dialvarezs @jfy133
Update modules/local/busco.nf
07e3569 
Co-authored-by: James A. Fellows Yates <[email protected]>
@dialvarezs
Remove commented code
e346460
@dialvarezs dialvarezs merged commit 4fa99d3 into nf-core:dev Mar 13, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@prototaxites prototaxites prototaxites left review comments

@jfy133 jfy133 jfy133 approved these changes

Assignees

No one assigned

Labels

ready to review

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@dialvarezs @prototaxites @jfy133