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

Make bindings accessible in SharedLibrary classes. #519

Merged

Conversation

nestoracunablanco
Copy link
Contributor

For the library loading a Pogo is instantiated, which has a different implementation while getting an object.

This POGO does not read the bindings. In the PogoMetaClassGetPropertySite the implementation can be found. This throws a runtime exception, which suits well for the common purposes, but not when we try to emulate the Jenkins shared library behaviour.

So the workaround is to add the bindings as properties dynamically inside InterceptingGCL.

This change tries to solve the issue #478

  • [ X] Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
  • [ X] Ensure that the pull request title represents the desired changelog entry
  • [ X] Please describe what you did
  • [ X] Link to relevant issues in GitHub or Jira
  • [ X] Link to relevant pull requests, esp. upstream and downstream changes
  • [ X] Ensure you have provided tests - that demonstrates feature works or fixes the issue

For the library loading a Pogo is instantiated, which has a different implementation while getting an object.

This POGO does not read the bindings. In the PogoMetaClassGetPropertySite the implementation can be found. This throws a runtime exception, which suits well for the common purposes, but not when we try to emulate the Jenkins shared library behaviour.

So the workaround is to add the bindings as properties dynamically inside InterceptingGCL.
Copy link
Contributor

@stchar stchar left a comment

Choose a reason for hiding this comment

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

Please add on more test scenario.

@nestoracunablanco
Copy link
Contributor Author

Hi @stchar, could you please take a look at my changes? Is that what you requested?
Thanks in advance!

Copy link
Contributor

@stchar stchar left a comment

Choose a reason for hiding this comment

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

Looks good. Thank you!

@nre-ableton nre-ableton merged commit e04cc9b into jenkinsci:master May 16, 2022
@nestoracunablanco nestoracunablanco deleted the bugfix/paramsNotAccesible branch May 17, 2022 06:16
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.

3 participants