-
-
Notifications
You must be signed in to change notification settings - Fork 33
Fix webapp resource lookup #76
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
Conversation
|
The last commit fixes the URL like http://localhost:8080/plugin/credentials/help/domain/name.html My apologies for getting this wrong, I have no idea how these slipped through. |
| </resource> | ||
| <resource> | ||
| <directory>plugins/credentials-plugin/src/main/webapp</directory> | ||
| <targetPath>webapp</targetPath> |
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.
So the interesting bit is, as currently proposed, plugin webapp/ resources would like in a subdirectory, while core webapp/ resources would not. This is due to #getPluginResource being exclusively used for plugin webapp resources, while #getResource is used for other resources as well.
|
Conflicted because the other PR was squash-merged, with a regular merge commit this would not have happened. |
|
It might because I made some changes at #77 |
|
Let me fix this conflict. |
LinuxSuRen
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.
LGTM
Git understand commit history and can easily merge without conflict — unless you use squash-merges or rebase-merges, as those will create different commits, and histories diverging so that the tool cannot automatically merge anymore. |
|
Yes, I understand this. I just want to keep a clean commit log. |
Currently, Core webapp resources such as
/help/system-config/master-slave/usage.htmldo not work. This was an oversight in the previous PR. This change should address the problem.@LinuxSuRen Please confirm.