jenkinsci / extension-filter-plugin

https://plugins.jenkins.io/extension-filter/
MIT License
2 stars 7 forks source link

Avoid Guice circular issue and implement test to ensure extension is activated #69

Closed jonesbusy closed 6 months ago

jonesbusy commented 6 months ago

Testing done

Automated tests

### Submitter checklist
- [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
- [ ] Link to relevant issues in GitHub or Jira
- [ ] Link to relevant pull requests, esp. upstream and downstream changes
- [ ] Ensure you have provided tests - that demonstrates feature works or fixes the issue
jonesbusy commented 6 months ago

Can you take a look @sridamul

sridamul commented 6 months ago

Can you also remove the validation? or need to figure out other way to display the message correctly.

sridamul commented 6 months ago

Can we use the extension from the extension Indexer to validate class? If possible, we can use it after the completion of this GSoC'24 project

jonesbusy commented 6 months ago

Can you also remove the validation? or need to figure out other way to display the message correctly.

https://github.com/jenkinsci/extension-filter-plugin/pull/70

By reading doc this can be achieved using an other class loader to check class existence

jonesbusy commented 6 months ago

Can we use the extension from the extension Indexer to validate class? If possible, we can use it after the completion of this GSoC'24 project

Not sure what you mean. But we are talking about 2 different tool here (One plugin that can disable extension on a running controller and one tool that just display the available extension across all plugin)

sridamul commented 6 months ago

Not sure what you mean. But we are talking about 2 different tool here (One plugin that can disable extension on a running controller and one tool that just display the available extension across all plugin)

I thought that one can check whether the class names are available in the extension indexer. If it presents, then it is an extension and can be indicated with correct validation

As you suggested

https://github.com/jenkinsci/extension-filter-plugin/pull/70 By reading doc this can be achieved using an other class loader to check class existence

this is better approach.

sridamul commented 6 months ago

Can you take a look @sridamul

tested the code locally (both descriptor and extension) LGTM!

sridamul commented 6 months ago

If possible can you remove the default="&{null}"in the https://github.com/jenkinsci/extension-filter-plugin/blob/main/src/main/resources/org/jenkinsci/plugins/ConfigurableExtensionFilter/global.jelly#L12 as suggested by 'Daniel Beck'