- 
                Notifications
    
You must be signed in to change notification settings  - Fork 338
 
Add external reader support in pkl-gradle #1067
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
base: main
Are you sure you want to change the base?
Conversation
498201d    to
    fc0567e      
    Compare
  
    fc0567e    to
    a18074d      
    Compare
  
    | 
           @HT154 can we add some test coverage for this? I think this would involve moving  Hint: the Groovy syntax for maps is  externalModuleReaders = ["foo": new org.pkl.gradle.ExternalReader("bar")] | 
    
e6b911e    to
    f6d3224      
    Compare
  
    | @Nested | ||
| @Optional | ||
| public abstract MapProperty<String, ExternalReader> getExternalModuleReaders(); | ||
| 
               | 
          ||
| @Nested | ||
| @Optional | ||
| public abstract MapProperty<String, ExternalReader> getExternalResourceReaders(); | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These should be marked as @Input properties, not @Nested. The @Nested annotation in Gradle is used to make Gradle process values of a map property as beans. All @Nested properties are treated as @Input, so it will work as is, but semantically values of this map are just data carriers without any annotations which do not require further processing, so I think that @Input would be more appropriate here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @netvl! Do you know of any documentation that covers these subtleties? I wasn't able to find anything when I searched.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is all described in Gradle docs, but it may be hard to find, especially since Gradle docs constantly evolve. Most of it comes with experience with writing plugins and sifting through tons of github tickets in the Gradle repo :)
On this particular thing, I believe, the description is here: https://docs.gradle.org/current/userguide/incremental_build.html#sec:task_input_output_annotations
818061b    to
    5b2a4c8      
    Compare
  
    5b2a4c8    to
    a1565c2      
    Compare
  
    There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
| 
           It looks like this hangs right now for reasons I'm not understanding. It's intermittent on *nix systems and seemingly consistent on Windows (but I don't have a system to test that locally). Will likely need a hand debugging this.  | 
    
No description provided.