Handle presence of index.jelly differently

[twasyl]

Signed-off-by: Thierry Wasylczenko [email protected]

Currently if there is no src/main/resources/index.jelly file in the Jenkins plugin sources, mvn hpi:run will fail the build indicating the file must be created using the POM's <description>. This behaviour has been introduced in #302. As I like to enforce the creation of index.jelly, the current approach:

• assumes there is a <description> in the POM
• lead to think you have to delete the <description> from your POM

IMO this could be improved for plugins author's UX. If the description is present but the the file is not, I assume the author:

• doesn't need to create a file when it's content is predictive (because we have the description)
• doesn't want the content of index.jelly to be different from the description

This PR aims to provide the following behaviours:

• In case the index.jelly file doesn't exist and the description is provided, create the file under target/classes and log a warning

• In case the index.jelly file doesn't exist and the description is not provided, then throw an error to enforce the creation of the file by the author

• In case the index.jelly file is provided, use it

• Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!

• Ensure that the pull request title represents the desired changelog entry

• 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

[Vlatombe]

I'm not at ease thinking that hpi:hpi can somehow write under src/ directory.

Such goal should always be reproducible, and here if you call it twice, the first call will fail, and the second one will succeed.

Adding a separate goal to implement such behaviour would be acceptable IMO.

[daniel-beck]

I'm not sure generating sources outside directories expected to contain something like that is expected. Since users will need to git add the file and commit it, there's no actual automation here.

I could see an argument for generating the index.jelly from POM description in the target/ though, OTOH a case could be made that developers are expected to provide a better description, e.g. (limited) HTML formatting is possible here.

[twasyl]

Sorry for maybe being unclear: the generated file is in target/classes, not in any source directory, see

[daniel-beck]

Sorry for maybe being unclear: the generated file is in target/classes, not in any source directory, see

Ah, I was confused by the path in the error message 🤦

[Vlatombe]

Sorry for maybe being unclear: the generated file is in target/classes, not in any source directory, see

I got confused as well. Looks okay to me. CI still needs fixing.

[twasyl]

I don't understand the current tests failures as in the output, there is the message which is checked in the logs 🤷

[jglick]

No strong opinion, but

a case could be made that developers are expected to provide a better description

I think this is a good opportunity to just have the developer take a couple minutes to write the description they wish to show to users, possibly with formatting. Most plugins already have an index.jelly so this should only affect a small minority anyway.

[jglick]

This should be setting an XML encoding line.

<?xml version="1.0" encoding="UTF-8"?>

[twasyl]

The previous error suggested:

throw new MojoFailureException("Missing " + indexJelly + ". Delete any <description> from pom.xml and create src/main/resources/index.jelly:\n" +
                    "<?jelly escape-by-default='true'?>\n" +
                    "<div>\n" +
                    "    The description here…\n" +
                    "</div>");

🤣

[twasyl]

Fixed it

[jtnord]

I am not sure why we would want this behaviour at all.

the Update center will use the description if the index.jelly is not found (as will the plugin manager if I am not mistaken).

So all this does is make a currently failing (has a description but no index.jelly) build pass.

You could just as easily do that by not failing in the first place (change the failure to a warning if there is no index.jelly) and avoid this generation.

but if the point is that people do not pay attention to warnings - why would they pay attention to the new warning here and provide something better?

[twasyl]

Closing as stale. Nobody approved it in a year.