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

Remove Side-files Support #636

Closed
wants to merge 5 commits into from

Conversation

ahmed-kamal2004
Copy link
Contributor

👋

  • According to issue Drop support for side-files #483, side-files feature is not used in Jenkins, and it results in wasting a part of time crawling the filesystem searching for index.html side-files

Screenshot from 2025-01-30 01-36-18

So I removed that snippet of code
Dispatcher d = IndexHtmlDispatcher.make(webApp.context, clazz);
if (d!=null)
dispatchers.add(d);

this removes support for web-inf/side-files/index.html pages


  • update maven-example

Justification: I tried to use jetty with it before updating, but that resulted in various problems, now anyone can run it using mvn jetty:run

example when sending request to local-instance of the example at localhost:9090/items/
Screenshot from 2025-01-31 16-52-18


  • use discover instead of 'discoverExtensions' following the TODO in the code.

  • Note: afaik jenkins now is using only (html-'jelly'-'groovy') files, there is no jsp files
    so we can't remove the facets support
    Screenshot from 2025-01-31 17-20-29
    Screenshot from 2025-01-31 17-21-41
    Screenshot from 2025-01-31 17-22-08
    Screenshot from 2025-01-31 17-22-37

some snippets showing the used extensions in jenkins, and there is no .jsp

@ahmed-kamal2004
Copy link
Contributor Author

ahmed-kamal2004 commented Feb 3, 2025

@timja @jglick
please review if this pull request can be merged or need adjustments, thanks

@ahmed-kamal2004 ahmed-kamal2004 closed this by deleting the head repository Feb 3, 2025
@jglick
Copy link
Member

jglick commented Feb 4, 2025

Superseded by #637?

@ahmed-kamal2004
Copy link
Contributor Author

Superseded by #637?

Yes, as I am seeking a better commits history in stapler repo

@jglick
Copy link
Member

jglick commented Feb 4, 2025

For the record, you can just force-push to an existing pull request branch if you preferred to rebase your own commits for whatever reason.

@ahmed-kamal2004
Copy link
Contributor Author

Thanks @jglick ⚡️ , I will consider this info in my consideration in the comming contribution 🚀
regarding #637 is there any comments on it ? or a change needed ?

@jglick
Copy link
Member

jglick commented Feb 7, 2025

Sorry, have not had a chance to look at it, have several hundred GitHub notifications to sort through! Others might get to it before me.

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.

2 participants