-
Notifications
You must be signed in to change notification settings - Fork 6
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
Plugin refresh and cd #6
Conversation
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.
🏇
src/main/resources/index.jelly
Outdated
This plugin provides support for iterating through all the <a href="https://javadoc.jenkins.io/jenkins/model/Nodes.html">Node</a> | ||
instances that are in use by <code>Jenkins</code>, even those <code>Node</code> instances that are not traditionally attached to Jenkins. The API exposed by this | ||
plugin can be used by cloud provider plugins to identify unused provisioned resource. | ||
</description> |
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.
Inconsistent indentation:
</description> | |
</description> |
Also, per https://github.com/jenkinsci/jenkins/blob/44d0496f38584dcc9fffc42de4104b66753649a3/core/src/main/resources/hudson/PluginManager/installed.jelly#L110-L129 the description from src/main/resources/index.jelly
is preferred over the <description>
from pom.xml
("last resort" according to the Jenkins core source code), making the <description>
from pom.xml
now redundant. That is why https://github.com/jenkinsci/maven-hpi-plugin/blob/694a97af11fdc7b841ee220c17ed64dfaffe08b0/src/main/java/org/jenkinsci/maven/plugins/hpi/HpiMojo.java#L139-L143 says:
Delete any
<description>
frompom.xml
and createsrc/main/resources/index.jelly
This PR creates src/main/resources/index.jelly
, but it does not delete the <description>
from pom.xml
.
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.
Description is a maven thing that has uses outside Jenkins, so I explicitly ignored that.
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.
It does have theoretical uses outside of Jenkins, but I don't think there are any such uses in practice, and I think having the same information copied and pasted in multiple places slightly increases maintenance burden, so for that reason and consistency with existing precedent I still have a slight preference for deleting the <description>
from pom.xml
, but I don't feel strongly about this.
I do still think that the indentation should be made consistent by deleting the spaces, at the beginning of the line, but I wouldn't block this PR over some whitespace. I already approved the PR.
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.
the invalid html has been fixed description
-> div
along with the indent in 03e0f31
@@ -0,0 +1,13 @@ | |||
name: cd |
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.
this is not a dependabot file... #7
Update the plugin baseline and enable CD
depends on jenkins-infra/repository-permissions-updater#2722