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.

Sign up for GitHub

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

Jump to bottom

feature: new dsl syntax to explicit declaration of plugin extensions #2820

Merged
merged 11 commits into from
May 4, 2022
Merged

feature: new dsl syntax to explicit declaration of plugin extensions #2820

merged 11 commits into from
May 4, 2022

Conversation

jorgeaguileraseqera
Copy link
Contributor

This PR implements the new import plugin functionality by reusing the IncludeDef process creating a Map of alias and methods implemented into the plugin

it intercepts the missing static method into the Channel class and reroutes the call to the corresponding ExtensionPoint function

closes #2804

pditommaso
pditommaso requested changes Apr 28, 2022
View reviewed changes
Copy link
Member

@pditommaso pditommaso left a comment

Choose a reason for hiding this comment

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

Ok, it looks very good. Mostly think it would be better to comment a bit more especially groovy magic extension methods.

Also, we can get rid of the scope mechanism, and therefore of this

nextflow/modules/nextflow/src/main/java/groovy/runtime/metaclass/NextflowDelegatingMetaClass.java

Lines 69 to 76 in 6b45b50

@Override
public Object getProperty(Object object, String property) {
// check if the property name is
ChannelFactory ext = Channel.class.equals(object) && plugin!=null
? plugin.getChannelFactory(property)
: null;
return ext != null ? ext : delegate.getProperty(object, property);
}

pditommaso
pditommaso requested changes May 2, 2022
View reviewed changes
Copy link
Member

@pditommaso pditommaso left a comment

Choose a reason for hiding this comment

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

Ok. I've removed the code related to the scope handling. Since it was only used by the SQL plugin, we can leave with a breaking chance and simplify the implementation.

I've left some other comments

modules/nextflow/src/main/groovy/nextflow/extension/ChannelExtensionDelegate.groovy Show resolved Hide resolved
modules/nextflow/src/main/groovy/nextflow/extension/ChannelExtensionDelegate.groovy Outdated Show resolved Hide resolved
modules/nf-commons/build.gradle Outdated Show resolved Hide resolved
pditommaso and others added 8 commits May 4, 2022 11:28
@pditommaso @jorgeaguileraseqera
Add support for module assets WIP [ci fast]
aea7acc 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@jorgeaguileraseqera
feature: new dsl syntax to explicit declaration of plugin extensions
0b730a5 
closes #2804

Signed-off-by: Jorge Aguilera <jorge.aguilera@seqera.io>
@jorgeaguileraseqera
feature: new dsl syntax to explicit declaration of plugin extensions
e96ec05 
add comments and minor changes

Signed-off-by: Jorge Aguilera <jorge.aguilera@seqera.io>
@jorgeaguileraseqera
feature: new dsl syntax to explicit declaration of plugin extensions
9647aab 
upgrade jgit

Signed-off-by: Jorge Aguilera <jorge.aguilera@seqera.io>
@jorgeaguileraseqera
feature: new dsl syntax to explicit declaration of plugin extensions
ef3d02a 
revert to jgit 5.2.1

Signed-off-by: Jorge Aguilera <jorge.aguilera@seqera.io>
@pditommaso @jorgeaguileraseqera
Remove deprecated plugins code [ci fast]
2dcedc0 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@jorgeaguileraseqera
feature: new dsl syntax to explicit declaration of plugin extensions
e2da349 
use nf-hello instead nf-sql to test dsl

Signed-off-by: Jorge Aguilera <jorge.aguilera@seqera.io>
@jorgeaguileraseqera
feature: new dsl syntax to explicit declaration of plugin extensions
c9ab37e 
fix test

Signed-off-by: Jorge Aguilera <jorge.aguilera@seqera.io>
@jorgeaguileraseqera jorgeaguileraseqera force-pushed the 2804-dsl2-plugin-extension-explicit-declaration branch from ec1ef2b to c9ab37e Compare May 4, 2022 09:55
@pditommaso
Minor refactoerting [ci fast]
769fdda 
Signed-off-by: Paolo Di Tommaso <paolo.ditommaso@gmail.com>
@pditommaso
Naming refactor + comments
fbb6302
@pditommaso
Copy link
Member

I've made naming refactoring and added some comments. Think we can merge if all tests are ok

@pditommaso
Copy link
Member

We still have the double build 🤦

@pditommaso pditommaso merged commit bfc4a06 into master May 4, 2022
@pditommaso pditommaso deleted the 2804-dsl2-plugin-extension-explicit-declaration branch May 4, 2022 17:12
@pditommaso
Copy link
Member

pditommaso commented May 4, 2022 •
edited
Loading

Done. Thanks for the great job!

@pditommaso
Copy link
Member

Released along with version 22.05.0-edge

@pditommaso pditommaso added this to the 22.10.0 milestone May 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bentsherman bentsherman Awaiting requested review from bentsherman

@pditommaso pditommaso Awaiting requested review from pditommaso

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
22.10.0
Development

Successfully merging this pull request may close these issues.

DSL2 plugin extension explicit declaration
2 participants
@jorgeaguileraseqera @pditommaso