Change skin for mvn site report to use fluido#731
Conversation
|
'target/site/property-updates-report.html' is still broken (missing trailing '}'. And not really sure that I like the image at the top of the site reports. Mostly just takes up room. I had already had updated to '-M3' version of the plugin but hadn't pushed yet and if it doesn't work without a skin, I obviously hadn't run 'mvn site', but I don't see any place where the '-M2' version was failing that '-M3' also isn't still failing. (E.g., the tag library documentation, which I what I think we miss most still isn't working as you mentioned in a private email.) Anyway, if you create a GitHub issue and describe what reports this actually broken with it (so I can reference it in the next release notes), I will merge this. BTW, you would think they would have had required the site.xml file to go under 'src/main/assembly' or maybe under 'src/main/resources' rather than 'src/site'. Adding more directories for no good reason (versus using an existing one) just means it's yet another place to forget about things. Not your fault, I know, but 'src/main/assembly' would have been better IMO. |
|
Oh, one more thing. if we're going to have a logo on the 'mvn site' reports, I'd prefer to use the ESAPI logo instead (attached), scaled down of course. |
|
Tag pages were broken or blank before if I recall. I see no reason not to
merge.
…On Tue, Jul 26, 2022 at 9:28 PM Kevin W. Wall ***@***.***> wrote:
'target/site/property-updates-report.html' is still broken (missing
trailing '}'. And not really sure that I like the image at the top of the
site reports. Mostly just takes up room. I had already had updated to '-M3'
version of the plugin but hadn't pushed yet and if it doesn't work without
a skin, I obviously hadn't run 'mvn site', but I don't see any place where
the '-M2' version was failing that '-M3' also isn't still failing. (E.g.,
the tag library documentation, which I what I think we miss most still
isn't working as you mentioned in a private email.)
Anyway, if you create a GitHub issue and describe what reports this
actually broken with it (so I can reference it in the next release notes),
I will merge this.
BTW, you would think they would have had required the site.xml file to go
under 'src/main/assembly' or maybe under 'src/main/resources' rather than
'src/site'. Adding more directories for no good reason (versus using an
existing one) just means it's yet another place to forget about things. Not
your fault, I know, but 'src/main/assembly' would have been better IMO.
—
Reply to this email directly, view it on GitHub
<#731 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGFWBOQBXBDEMMLPW73GD3VWCGFFANCNFSM54TQ4EBA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Please create a GitHub issue, assign it to yourself, and then mention this PR in it and I'll do the merge. I just don't like to have commits / PRs not associated with GitHub issues (except for minor bookkeeping issues during releases, like removing '-SNAPSHOT', etc.) because I document the GitHub issues that were closed in release notes, not the specific PRs or commits and IMO, that's a more useful synopsis than just (say) looking at a change log. |
|
On a previous episode, @davewichers wrote:
These tag pages are still broken. The only difference I see is that the indexes for:
no longer appear in the sidebar navigation panel so you just don't notice. I will write up a GitHub issue, but aside from some minuscule issues such as a messed up copyright notice (the year appeared as "Copyright © ${currentYear} ..."), I really don't see what was fixed. So to me this PR just looks like it changes the look-and-feel (admittedly, to one that appears a bit slicker) but really doesn't fix anything. So I need to know what to put into the the GitHub issue that I will create. So, I don't even know if this would be considered primarily a 'bug' or an 'enhancement' and thus I need some help in knowing what to put into the GH issue. Also, note that with this PR merged, there will also a be downside of 'mvn site' no longer generating that sidebar navigation index which means that I had to create GitHub issue #733 so I wouldn't forget about the tag documentation not working. (Out-of-sight, out-of-mind.) |
| @@ -1,5 +1,5 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> | |||
There was a problem hiding this comment.
Question for you @davewichers - This is just an XML namespace and often IRL, these URLs do not even exist and would return a 404. So if we were to change them to https would the schema validation still work or is there a chance it could break something. TBH, I've never seen https:// schema used in XML namespace and I'd be concerned if someone wrote a parser to do schema validations of Maven pom files (like might be in some IDEs), that this change could break something.
There was a problem hiding this comment.
@davewichers - I'm going to reject this particular change as it causes an error / warning in Eclipse and perhaps other zIDEs as well. See attached pic for example
.
There was a problem hiding this comment.
I have created a GH issue for this (issue #734), but you have to promise me to review it for accuracy before I merge this PR.
Also, please undo the changes to the XML namespace in the pom.xml (see previous comment.) Thanks.
| @@ -1,5 +1,5 @@ | |||
| <?xml version="1.0" encoding="UTF-8"?> | |||
| <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/maven-v4_0_0.xsd"> | |||
There was a problem hiding this comment.
@davewichers - I'm going to reject this particular change as it causes an error / warning in Eclipse and perhaps other zIDEs as well. See attached pic for example
.
|
I don't have time/interest in creating an issue. Please do so yourself so
you can then reference it in your acceptance of this commit. I already
spent an hour+ on this.
…-Dave
On Tue, Jul 26, 2022 at 9:28 PM Kevin W. Wall ***@***.***> wrote:
'target/site/property-updates-report.html' is still broken (missing
trailing '}'. And not really sure that I like the image at the top of the
site reports. Mostly just takes up room. I had already had updated to '-M3'
version of the plugin but hadn't pushed yet and if it doesn't work without
a skin, I obviously hadn't run 'mvn site', but I don't see any place where
the '-M2' version was failing that '-M3' also isn't still failing. (E.g.,
the tag library documentation, which I what I think we miss most still
isn't working as you mentioned in a private email.)
Anyway, if you create a GitHub issue and describe what reports this
actually broken with it (so I can reference it in the next release notes),
I will merge this.
BTW, you would think they would have had required the site.xml file to go
under 'src/main/assembly' or maybe under 'src/main/resources' rather than
'src/site'. Adding more directories for no good reason (versus using an
existing one) just means it's yet another place to forget about things. Not
your fault, I know, but 'src/main/assembly' would have been better IMO.
—
Reply to this email directly, view it on GitHub
<#731 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABGFWBOQBXBDEMMLPW73GD3VWCGFFANCNFSM54TQ4EBA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
@davewichers wrote:
Huh? I already created the issue for this (#734, no closed) and already merged this PR. Looks like you are maybe responding to old emails from your inbox??? Anyhow, this is all taken care of. |
The 4.0.0-Mx version of the -site plugin is broken if you don't specify a recent version of a skin it supports. So I am upgrading that plugin, specifying the skin version in the pom, and also created a custom site.xml based on AntiSamy's that references that skin. I also turned off the tag reports since they are all broken currently.