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

Lint warning when using a template #1558

Closed
Midnighter opened this issue May 10, 2022 · 11 comments
Closed

Lint warning when using a template #1558

Midnighter opened this issue May 10, 2022 · 11 comments
Labels
bug Something isn't working
Milestone

Comments

@Midnighter
Copy link
Contributor

Description of the bug

When using a template instead of a script block inside of a process, I get the following lint warning:

Process name not used for versions.yml

which seems like a parsing bug of the linter.

Command used and terminal output

nf-core modules lint sratools/prefetch  # On currently open PR.

System information

nf-core, version 2.4.dev0

@Midnighter Midnighter added the bug Something isn't working label May 10, 2022
@Midnighter
Copy link
Contributor Author

See nf-core/modules#1631 and the failing lint action.

@ewels
Copy link
Member

ewels commented May 12, 2022

(exact!) duplicate of #1557 😉

@ewels ewels closed this as completed May 12, 2022
@Midnighter
Copy link
Contributor Author

Actually no, different lint warning.

@Midnighter Midnighter reopened this May 12, 2022
@ewels
Copy link
Member

ewels commented May 12, 2022

Apologies, rushing and misread 🙏🏻

# On currently open PR.

Can you avoid this in future issues please? I rarely get to them in time and usually have to spend ages figuring out what date the issue was posted and digging through git history to guess at which file was the one that caused the error 😅

Links to actions runs / file permalinks is best as it's quick to replicate then 👌🏻

@ewels
Copy link
Member

ewels commented May 12, 2022

ok I should stop working and go and eat. Just realised that your comment immediately after opening the issue was exactly that 👍🏻

@ewels
Copy link
Member

ewels commented May 12, 2022

...except the PR and failing lint action has the error from the other issue which I thought that you'd duplicated:

╭──────────────────────────────────────────────────────────────────────────────╮
│ [!] 1 Test Failed                                                            │
╰──────────────────────────────────────────────────────────────────────────────╯
╭──────────────────────────────────────────┬─────────────────┬─────────────────╮
│ Module name                              │ File path       │ Test message    │
├──────────────────────────────────────────┼─────────────────┼─────────────────┤
│ sratools/prefetch                        │ modules/sratoo… │ when: condition │
│                                          │                 │ has too many    │
│                                          │                 │ lines           │
╰──────────────────────────────────────────┴─────────────────┴─────────────────╯

I guess the force-push in that PR dropped reference to the original CI tests where this was happening.

Could you link to a reproducible example please? 🙏🏻

@ewels ewels added this to the 2.4 milestone May 12, 2022
@Midnighter
Copy link
Contributor Author

I'll provide more stable links in future, sorry. The warning is right there, though https://github.com/nf-core/modules/runs/6368191141?check_suite_focus=true#step%3A9%3A29=

@Midnighter
Copy link
Contributor Author

To reproduce:

pip install -U https://github.com/nf-core/tools/archive/dev.zip

and in the modules repo:

nf-core modules lint custom/sratoolsncbisettings

@ewels
Copy link
Member

ewels commented May 12, 2022

Ahhh warning not error, that's why I didn't see it. Sorry 🤦🏻

@mirpedrol
Copy link
Member

It seems that when using a template the linting checks for the script section (which is empty in that case) are also performed, thus the error.
Should we only perform the checks when there is a script section?
We could also include a template section to perform the linting, but I guess it will be varying a lot between different templates, won't it?

ewels added a commit to ewels/nf-core-tools that referenced this issue May 13, 2022
Basically, skip this step if using shell instead.
Fixes nf-core#1558
@ewels ewels closed this as completed May 13, 2022
@ewels
Copy link
Member

ewels commented May 13, 2022

We could also include a template section to perform the linting, but I guess it will be varying a lot between different templates, won't it?

Yeah I wondered the same but I'm not sure that we can do much with this. I did wonder about checking for the version etc. but it would be fairly complex code and probably quite liable to break. There aren't loads of modules taking this approach so I'm not in a rush to do this for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants