-
Notifications
You must be signed in to change notification settings - Fork 257
Add DeclarativeDirective extension point for adding directives #260
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: master
Are you sure you want to change the base?
Conversation
abayer
commented
Apr 4, 2018
- JENKINS issue(s):
- n/a
- Description:
- New directives must provide a method for parsing from Jenkinsfile syntax, a JSON schema, a runtime model class, validation for the parsed AST model, and a method for transforming from the parsed AST model to the runtime model.
- Initially, will only provide support for preprocessing directives - which will provide a way to transform the full AST model after parsing and before validation and transformation. This will enable plugin authors to contribute directives that can be replaced by predetermined content before validation starts.
- Documentation changes:
- Actually, this makes me think that developer docs for all the Declarative extension points would be a good idea...
- Users/aliases to notify:
- ...
|
Note - this is a work-in-progress. I need to add tests and do some prototyping for a use case I have in mind to make sure this implementation will actually do the trick. I also need to decide if it's actually worth it to keep this as general as it is, or if it makes more sense to just provide a pre-processing directive extension point... |
c785390 to
923591b
Compare
|
By just reading the title I fear this could lead to the "declarative model" getting out of hand if there is free reign for other plugins to add to the structure. |
52e91f2 to
274a0c8
Compare
a3902e3 to
1c75a07
Compare
I only looked at the last commit and got confused
f0c2e46 to
aefad28
Compare
New directives must provide a method for parsing from Jenkinsfile syntax, a JSON schema, a runtime model class, validation for the parsed AST model, and a method for transforming from the parsed AST model to the runtime model. Initially, will only provide support for preprocessing directives - which will provide a way to transform the full AST model after parsing and before validation and transformation. This will enable plugin authors to contribute directives that can be replaced by predetermined content before validation starts.
Useful for something that might want to extend ModelParser.
logging out of parsePipelineStep into new parseValidateAndTransformPipeline method.
aefad28 to
c254c76
Compare
|
@abayer This seems like it could really use a JEP. |
rsandell
left a comment
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.
for being a new API the JavaDoc is lacking.
| private static Logger LOGGER = Logger.getLogger(DeclarativeDirectiveDescriptor.class.getName()); | ||
|
|
||
| @Nonnull | ||
| public abstract String getName(); |
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.
You seem to be using this in quite a lot of places that confuses me on the distinction between name and displayName.
But I can't come up with a better word for this either :)
|
@abayer Any chance you can resolve the merge conflicts here? Should this remain open? |