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

cylc lint - missing jinja2 shebang excessive warnings? #5541

Closed
ColemanTom opened this issue May 16, 2023 · 4 comments · Fixed by #5549
Closed

cylc lint - missing jinja2 shebang excessive warnings? #5541

ColemanTom opened this issue May 16, 2023 · 4 comments · Fixed by #5549
Assignees
Labels
bug Something is wrong :(
Milestone

Comments

@ColemanTom
Copy link
Contributor

Description

If you run cylc lint and it finds multiple .cylc files, it will want a #!jinja2 line at the top of each of them if it detects jinja2 in there. Is this required, or should it only be required in the flow.cylc file? Looking at #4917 - that reads as though it should be like I suggest, only for the top level file.

Reproducible Example

touch flow.cylc
echo '{{a}}' > include.cylc
cylc lint
[S008] include.cylc:1: jinja2 found: no shebang (#!jinja2)

Expected Behaviour

No warning as it is not the flow.cylc file.

@ColemanTom ColemanTom added the bug Something is wrong :( label May 16, 2023
@oliver-sanders oliver-sanders added this to the cylc-8.2.0 milestone May 16, 2023
@wxtim
Copy link
Member

wxtim commented May 16, 2023

Cylc lint is meant to be agnostic of what the top-level file is - unlike Cylc validate it's meant to look at all .cylc files, without paying attention to what is and is not included - this is to prevent issues cropping up later if an include is switched on.

I think, however, that it's probably reasonable to assume that if the top-level flow.cylc has a Jinja2 shebang then jinja2 is legitimate in all other files. If @oliver-sanders agrees with this assumption, I'll work out how to do it.

@oliver-sanders
Copy link
Member

Yes, the easy solution suggested above is to only apply this rule when the file name is flow.cylc which shouldn't be true for include files.

@oliver-sanders
Copy link
Member

This is actually the first point of #4917 (comment)

@MetRonnie
Copy link
Member

@wxtim Did #5549 address this?

@wxtim wxtim closed this as completed Jun 14, 2023
@MetRonnie MetRonnie linked a pull request Jun 14, 2023 that will close this issue
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something is wrong :(
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants