Skip to content

Cheaha strict syntax conversion #1047

Merged
jfy133 merged 4 commits intonf-core:masterfrom
U-BDS:master
Mar 19, 2026
Merged

Cheaha strict syntax conversion #1047
jfy133 merged 4 commits intonf-core:masterfrom
U-BDS:master

Conversation

@lianov
Copy link
Member

@lianov lianov commented Mar 17, 2026


name: cheaha profile

Please follow these steps before submitting your PR:

  • If your PR is a work in progress, include [WIP] in its title
  • Your PR targets the master branch
  • You've included links to relevant issues, if any

Issue #1020


@nvnieuwk : here is the requested PR. As a recap, currently the Nextflow VS Code plugin complains about line 66 with the message Config option 'process.queue' with type String cannot be assigned to value with type Closure

Please also note that I have not enabled .call() (yet) line 54 as I was attempting a fix for the warning above. Thanks!

As a note, if we can solve this here, I will still need to test the profile in our cluster before moving forward with the merge.

@pontus
Copy link
Collaborator

pontus commented Mar 18, 2026

I think it's the plugin that's wrong here, queue should be fine as a closure.

And you don't need/shouldn't do call() - that would cause evaluation at the wrong time.

You can go ahead and try this on your cluster.

@nvnieuwk
Copy link
Contributor

This will also still fail because the parameter is function closure now, you can directly move the closure to the input of queue instead of assigning it to a parameter. Assigning to a parameter here should be discouraged since the code is only used once

@pontus
Copy link
Collaborator

pontus commented Mar 18, 2026

But that closure is called with the expected parameters, so I'm not sure why it should fail?

I agree there's no point in having it in params though (and having it there likely gives warnings about it that should be dealt with by telling nf-schema to ignore it).

@lianov
Copy link
Member Author

lianov commented Mar 18, 2026

Hey all - excellent, I will make these changes and test it in our cluster and update this PR asap. Especial thanks for the clarification that I can safely ignore the Nexflow plug-in message (and before submitting this PR I did actually try to just directly provide the closure but the plug-in message was still there which generated confusion).

Either thanks! Will update here soon.

@lianov lianov changed the title WIP: Cheaha strict syntax conversion Cheaha strict syntax conversion Mar 19, 2026
@lianov
Copy link
Member Author

lianov commented Mar 19, 2026

@nvnieuwk : changes are made and tested with test profile of rnaseq and demo in our cluster. Both passed. Could you do a final review for a merge (I assume you can approve the PR?). Thanks!

Edit: some automated checks failing below but on other profiles - let me know if there is anything on this end to be addressed.

@jfy133
Copy link
Member

jfy133 commented Mar 19, 2026

@lianov can you try merging master into? I've just made a chance to the CI where failing other configs should not block your own test

Copy link
Contributor

@nvnieuwk nvnieuwk left a comment

Choose a reason for hiding this comment

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

LGTM!

@lianov
Copy link
Member Author

lianov commented Mar 19, 2026

@jfy133 : done - looks like some tests are still failing but the cheaha one went through (merging is still blocked though).

@jfy133
Copy link
Member

jfy133 commented Mar 19, 2026

@jfy133 : done - looks like some tests are still failing but the cheaha one went through (merging is still blocked though).

Yeah, this will take time - but at least we confirm Cheaha passes, so we can force merge for you!

@jfy133 jfy133 merged commit e6a60e5 into nf-core:master Mar 19, 2026
128 of 156 checks passed
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.

5 participants