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

Use the same stringInterceptor in declarative and scripted pipelines #460

Merged
merged 1 commit into from
Jan 3, 2022

Conversation

sams-gleb
Copy link

Basically just move the string method onto BasePipeline class as params could be used in the scripted pipeline, not only in declarative

Copy link
Contributor

@nre-ableton nre-ableton left a comment

Choose a reason for hiding this comment

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

Nice, thanks for the fix.

@nre-ableton nre-ableton merged commit faf3f28 into jenkinsci:master Jan 3, 2022
@reinholdfuereder
Copy link
Contributor

@nre-ableton I think the approach from #298 (comment) for "stringInterceptor" in combination with "parameters" and "withCredentials" steps (both having nested "string" steps) would be superior:

https://github.com/reinholdfuereder/JenkinsPipelineUnit/tree/parameters

@nre-ableton
Copy link
Contributor

@reinholdfeurerder I really have no idea what is going on in #298, which is why I didn't review it. In general, I'm not familiar with declarative pipelines, so I tend to stay away from that part of the codebase.

However, that PR doesn't make clear what the problem is nor why this is a good solution.

If you would like to submit a new PR with a similar solution that can better communicate the situation, I'd be happy to review it.

@reinholdfuereder
Copy link
Contributor

After starting with a new branch for a new PR I must admit that I re-evaluated the situation and must admit that I have to revoke my claim:

  • the advantage I had thought was superior is also a disadvantage, because the "parameters" element than contains duplicates of all the actual parameters (that are in the callstack right above)
    • in the following screenshot on the left my special overriding of "string" interceptor (and non-default "parameters" interceptor); and on the right after reverting them and using default "string" interceptor of Jenkins Pipeline Unit v1.14 (plus an additional default "parameters" interceptor => Support "parameters" in scripted pipeline job #495 with that and some new unit tests)

image

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.

None yet

3 participants