[ALL] Updates to commons-parent and commons-skin

classic Classic list List threaded Threaded
16 messages Options
Reply | Threaded
Open this post in threaded view
|

[ALL] Updates to commons-parent and commons-skin

Alex Herbert
The recent release of RNG has highlighted some issues with the commons
parent configuration for multi-module builds and a desirable change to
commons skin.

1. [parent] JaCoCo aggregate reports are included.

Aggregate reports show coverage of dependencies. Since most commons
components are stand-alone this should be disabled and the report set
updated to have non-aggregate reports.

An example of the output is shown for the recently release BCEL:

https://commons.apache.org/proper/commons-bcel/project-reports.html

The aggregate report is empty.


2. [parent] japicmp does not allow clean opt-in

The japicmp maven plugin is immature. If set to skip the report it still
creates a menu entry in the site reports page. See this release of
Collections:

https://commons.apache.org/proper/commons-collections/project-reports.html

The link for the japicmp report is a blank page.


For collections this could be fixed by running the report.

For a multi-module build using japicmp any module with no code (i.e. a
pom) has this empty entry if japicmp is enabled as the skip
functionality does not work.


The solution is to move the reporting section from the main
configuration into the japicmp profile. This allows opt-in on a
per-module basis to japicmp.


3. [skin] Customisation of the site <head> section

RNG uses formulas in the site documentation. Ideally this would be
rendered latex using MathJax [1].

This cannot be included in the site descriptor for the project as the
<head> entry for each page is generated by commons-skin. This adds
javascript to allow pretty print of code inside <pre "prettyprint">
tags. I would like to add a javascript tag to allow inclusion of MathJax.


I have rendered the RNG site using these changes and staged it here:

mvn package site site:stage -Dcommons.release.version=1.2
-DcomparisonVersion=1.2

https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html


Top level reports page does not have japicmp:

https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html

Components do:

https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html

(There are also no jacoco aggregate reports.)


You can view how MathJax is rendered on this page:

https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html

(search for 'will render an in-line formula')


AFAIK an update to commons-skin to include MathJax will not effect sites
that do not contain the \( or \[ characters in plain text on site pages.


I propose to:

- Update commons-skin to add a MathJax script to the <head> section

- Release commons-skin (last release was May 2015)

- Update commons-parent:

- Use the new commons-skin
- Remove JaCoCo aggregate reports
- Reconfigure japicmp for clean opt-in


Alex


[1] https://www.mathjax.org/



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

sebb-2-2
+1 for CP

CP versions are entirely optional; a new version is only used if a
component chooses to use it.

Is this also true of commons-skin?
If so, then +1

If not, then extra care needs to be taken to ensure backwards compatibilty.
There should probably be a formal vote on the actual changes in that case.


On Wed, 13 Nov 2019 at 13:32, Alex Herbert <[hidden email]> wrote:

> The recent release of RNG has highlighted some issues with the commons
> parent configuration for multi-module builds and a desirable change to
> commons skin.
>
> 1. [parent] JaCoCo aggregate reports are included.
>
> Aggregate reports show coverage of dependencies. Since most commons
> components are stand-alone this should be disabled and the report set
> updated to have non-aggregate reports.
>
> An example of the output is shown for the recently release BCEL:
>
> https://commons.apache.org/proper/commons-bcel/project-reports.html
>
> The aggregate report is empty.
>
>
> 2. [parent] japicmp does not allow clean opt-in
>
> The japicmp maven plugin is immature. If set to skip the report it still
> creates a menu entry in the site reports page. See this release of
> Collections:
>
> https://commons.apache.org/proper/commons-collections/project-reports.html
>
> The link for the japicmp report is a blank page.
>
>
> For collections this could be fixed by running the report.
>
> For a multi-module build using japicmp any module with no code (i.e. a
> pom) has this empty entry if japicmp is enabled as the skip
> functionality does not work.
>
>
> The solution is to move the reporting section from the main
> configuration into the japicmp profile. This allows opt-in on a
> per-module basis to japicmp.
>
>
> 3. [skin] Customisation of the site <head> section
>
> RNG uses formulas in the site documentation. Ideally this would be
> rendered latex using MathJax [1].
>
> This cannot be included in the site descriptor for the project as the
> <head> entry for each page is generated by commons-skin. This adds
> javascript to allow pretty print of code inside <pre "prettyprint">
> tags. I would like to add a javascript tag to allow inclusion of MathJax.
>
>
> I have rendered the RNG site using these changes and staged it here:
>
> mvn package site site:stage -Dcommons.release.version=1.2
> -DcomparisonVersion=1.2
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
>
>
> Top level reports page does not have japicmp:
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
>
> Components do:
>
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
>
> (There are also no jacoco aggregate reports.)
>
>
> You can view how MathJax is rendered on this page:
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
>
> (search for 'will render an in-line formula')
>
>
> AFAIK an update to commons-skin to include MathJax will not effect sites
> that do not contain the \( or \[ characters in plain text on site pages.
>
>
> I propose to:
>
> - Update commons-skin to add a MathJax script to the <head> section
>
> - Release commons-skin (last release was May 2015)
>
> - Update commons-parent:
>
> - Use the new commons-skin
> - Remove JaCoCo aggregate reports
> - Reconfigure japicmp for clean opt-in
>
>
> Alex
>
>
> [1] https://www.mathjax.org/
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Alex Herbert

On 13/11/2019 13:49, sebb wrote:
> +1 for CP
>
> CP versions are entirely optional; a new version is only used if a
> component chooses to use it.
>
> Is this also true of commons-skin?
> If so, then +1

cp bundles this site descriptor:

src/site.xml

which contains:

   <skin>
     <groupId>org.apache.commons</groupId>
     <artifactId>commons-skin</artifactId>
     <version>4.1</version>
   </skin>

So the skin is specified by cp.

You can only have 1 skin tag in a site descriptor.

I did a quick test to use a <skin> tag in the site descriptor for RNG.
This overrides the one in commons parent.

So cp could be updated without updating the skin.

I could update the skin and release it for use with RNG. Other
components could opt in to a change of the skin using their site
descriptor. When stability is known then cp could be updated for the skin.

Alex

>
> If not, then extra care needs to be taken to ensure backwards compatibilty.
> There should probably be a formal vote on the actual changes in that case.
>
>
> On Wed, 13 Nov 2019 at 13:32, Alex Herbert <[hidden email]> wrote:
>
>> The recent release of RNG has highlighted some issues with the commons
>> parent configuration for multi-module builds and a desirable change to
>> commons skin.
>>
>> 1. [parent] JaCoCo aggregate reports are included.
>>
>> Aggregate reports show coverage of dependencies. Since most commons
>> components are stand-alone this should be disabled and the report set
>> updated to have non-aggregate reports.
>>
>> An example of the output is shown for the recently release BCEL:
>>
>> https://commons.apache.org/proper/commons-bcel/project-reports.html
>>
>> The aggregate report is empty.
>>
>>
>> 2. [parent] japicmp does not allow clean opt-in
>>
>> The japicmp maven plugin is immature. If set to skip the report it still
>> creates a menu entry in the site reports page. See this release of
>> Collections:
>>
>> https://commons.apache.org/proper/commons-collections/project-reports.html
>>
>> The link for the japicmp report is a blank page.
>>
>>
>> For collections this could be fixed by running the report.
>>
>> For a multi-module build using japicmp any module with no code (i.e. a
>> pom) has this empty entry if japicmp is enabled as the skip
>> functionality does not work.
>>
>>
>> The solution is to move the reporting section from the main
>> configuration into the japicmp profile. This allows opt-in on a
>> per-module basis to japicmp.
>>
>>
>> 3. [skin] Customisation of the site <head> section
>>
>> RNG uses formulas in the site documentation. Ideally this would be
>> rendered latex using MathJax [1].
>>
>> This cannot be included in the site descriptor for the project as the
>> <head> entry for each page is generated by commons-skin. This adds
>> javascript to allow pretty print of code inside <pre "prettyprint">
>> tags. I would like to add a javascript tag to allow inclusion of MathJax.
>>
>>
>> I have rendered the RNG site using these changes and staged it here:
>>
>> mvn package site site:stage -Dcommons.release.version=1.2
>> -DcomparisonVersion=1.2
>>
>> https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
>>
>>
>> Top level reports page does not have japicmp:
>>
>> https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
>>
>> Components do:
>>
>>
>> https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
>>
>> (There are also no jacoco aggregate reports.)
>>
>>
>> You can view how MathJax is rendered on this page:
>>
>> https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
>>
>> (search for 'will render an in-line formula')
>>
>>
>> AFAIK an update to commons-skin to include MathJax will not effect sites
>> that do not contain the \( or \[ characters in plain text on site pages.
>>
>>
>> I propose to:
>>
>> - Update commons-skin to add a MathJax script to the <head> section
>>
>> - Release commons-skin (last release was May 2015)
>>
>> - Update commons-parent:
>>
>> - Use the new commons-skin
>> - Remove JaCoCo aggregate reports
>> - Reconfigure japicmp for clean opt-in
>>
>>
>> Alex
>>
>>
>> [1] https://www.mathjax.org/
>>
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: [hidden email]
>> For additional commands, e-mail: [hidden email]
>>
>>

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Gilles Sadowski-2
In reply to this post by sebb-2-2
Hello.

Le mer. 13 nov. 2019 à 14:49, sebb <[hidden email]> a écrit :
>
> +1 for CP
>
> CP versions are entirely optional;

Not "entirely".  [We already had this discussion.]
It happened that a fix for <something> was available in a newer CP but
that <something else> in CP broke some components (e.g. when CP
functionality is "enhanced" based on assumptions not valid for all
components).
Then, that *suddenly* unsupported component has to cherry-pick and
duplicate (in its local POM) what exists in that newer CP which it cannot
use anymore.

Gilles

> a new version is only used if a
> component chooses to use it.
>
> Is this also true of commons-skin?
> If so, then +1
>
> If not, then extra care needs to be taken to ensure backwards compatibilty.
> There should probably be a formal vote on the actual changes in that case.
>
>
> On Wed, 13 Nov 2019 at 13:32, Alex Herbert <[hidden email]> wrote:
>
> > The recent release of RNG has highlighted some issues with the commons
> > parent configuration for multi-module builds and a desirable change to
> > commons skin.
> >
> > 1. [parent] JaCoCo aggregate reports are included.
> >
> > Aggregate reports show coverage of dependencies. Since most commons
> > components are stand-alone this should be disabled and the report set
> > updated to have non-aggregate reports.
> >
> > An example of the output is shown for the recently release BCEL:
> >
> > https://commons.apache.org/proper/commons-bcel/project-reports.html
> >
> > The aggregate report is empty.
> >
> >
> > 2. [parent] japicmp does not allow clean opt-in
> >
> > The japicmp maven plugin is immature. If set to skip the report it still
> > creates a menu entry in the site reports page. See this release of
> > Collections:
> >
> > https://commons.apache.org/proper/commons-collections/project-reports.html
> >
> > The link for the japicmp report is a blank page.
> >
> >
> > For collections this could be fixed by running the report.
> >
> > For a multi-module build using japicmp any module with no code (i.e. a
> > pom) has this empty entry if japicmp is enabled as the skip
> > functionality does not work.
> >
> >
> > The solution is to move the reporting section from the main
> > configuration into the japicmp profile. This allows opt-in on a
> > per-module basis to japicmp.
> >
> >
> > 3. [skin] Customisation of the site <head> section
> >
> > RNG uses formulas in the site documentation. Ideally this would be
> > rendered latex using MathJax [1].
> >
> > This cannot be included in the site descriptor for the project as the
> > <head> entry for each page is generated by commons-skin. This adds
> > javascript to allow pretty print of code inside <pre "prettyprint">
> > tags. I would like to add a javascript tag to allow inclusion of MathJax.
> >
> >
> > I have rendered the RNG site using these changes and staged it here:
> >
> > mvn package site site:stage -Dcommons.release.version=1.2
> > -DcomparisonVersion=1.2
> >
> > https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
> >
> >
> > Top level reports page does not have japicmp:
> >
> > https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
> >
> > Components do:
> >
> >
> > https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
> >
> > (There are also no jacoco aggregate reports.)
> >
> >
> > You can view how MathJax is rendered on this page:
> >
> > https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
> >
> > (search for 'will render an in-line formula')
> >
> >
> > AFAIK an update to commons-skin to include MathJax will not effect sites
> > that do not contain the \( or \[ characters in plain text on site pages.
> >
> >
> > I propose to:
> >
> > - Update commons-skin to add a MathJax script to the <head> section
> >
> > - Release commons-skin (last release was May 2015)
> >
> > - Update commons-parent:
> >
> > - Use the new commons-skin
> > - Remove JaCoCo aggregate reports
> > - Reconfigure japicmp for clean opt-in
> >
> >
> > Alex
> >
> >
> > [1] https://www.mathjax.org/
> >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

sebb-2-2
In reply to this post by Alex Herbert
On Wed, 13 Nov 2019 at 14:02, Alex Herbert <[hidden email]> wrote:

>
> On 13/11/2019 13:49, sebb wrote:
> > +1 for CP
> >
> > CP versions are entirely optional; a new version is only used if a
> > component chooses to use it.
> >
> > Is this also true of commons-skin?
> > If so, then +1
>
> cp bundles this site descriptor:
>
> src/site.xml
>
> which contains:
>
>    <skin>
>      <groupId>org.apache.commons</groupId>
>      <artifactId>commons-skin</artifactId>
>      <version>4.1</version>
>

Might make sense to use a property for the version.


>    </skin>
>
> So the skin is specified by cp.
>
> You can only have 1 skin tag in a site descriptor.
>
> I did a quick test to use a <skin> tag in the site descriptor for RNG.
> This overrides the one in commons parent.
>
> So cp could be updated without updating the skin.
>
> I could update the skin and release it for use with RNG. Other
> components could opt in to a change of the skin using their site
> descriptor. When stability is known then cp could be updated for the skin.
>
>
Thanks for checking.

I am +1 on proceeding as you suggest.



> Alex
>
> >
> > If not, then extra care needs to be taken to ensure backwards
> compatibilty.
> > There should probably be a formal vote on the actual changes in that
> case.
> >
> >
> > On Wed, 13 Nov 2019 at 13:32, Alex Herbert <[hidden email]>
> wrote:
> >
> >> The recent release of RNG has highlighted some issues with the commons
> >> parent configuration for multi-module builds and a desirable change to
> >> commons skin.
> >>
> >> 1. [parent] JaCoCo aggregate reports are included.
> >>
> >> Aggregate reports show coverage of dependencies. Since most commons
> >> components are stand-alone this should be disabled and the report set
> >> updated to have non-aggregate reports.
> >>
> >> An example of the output is shown for the recently release BCEL:
> >>
> >> https://commons.apache.org/proper/commons-bcel/project-reports.html
> >>
> >> The aggregate report is empty.
> >>
> >>
> >> 2. [parent] japicmp does not allow clean opt-in
> >>
> >> The japicmp maven plugin is immature. If set to skip the report it still
> >> creates a menu entry in the site reports page. See this release of
> >> Collections:
> >>
> >>
> https://commons.apache.org/proper/commons-collections/project-reports.html
> >>
> >> The link for the japicmp report is a blank page.
> >>
> >>
> >> For collections this could be fixed by running the report.
> >>
> >> For a multi-module build using japicmp any module with no code (i.e. a
> >> pom) has this empty entry if japicmp is enabled as the skip
> >> functionality does not work.
> >>
> >>
> >> The solution is to move the reporting section from the main
> >> configuration into the japicmp profile. This allows opt-in on a
> >> per-module basis to japicmp.
> >>
> >>
> >> 3. [skin] Customisation of the site <head> section
> >>
> >> RNG uses formulas in the site documentation. Ideally this would be
> >> rendered latex using MathJax [1].
> >>
> >> This cannot be included in the site descriptor for the project as the
> >> <head> entry for each page is generated by commons-skin. This adds
> >> javascript to allow pretty print of code inside <pre "prettyprint">
> >> tags. I would like to add a javascript tag to allow inclusion of
> MathJax.
> >>
> >>
> >> I have rendered the RNG site using these changes and staged it here:
> >>
> >> mvn package site site:stage -Dcommons.release.version=1.2
> >> -DcomparisonVersion=1.2
> >>
> >> https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
> >>
> >>
> >> Top level reports page does not have japicmp:
> >>
> >>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
> >>
> >> Components do:
> >>
> >>
> >>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
> >>
> >> (There are also no jacoco aggregate reports.)
> >>
> >>
> >> You can view how MathJax is rendered on this page:
> >>
> >> https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
> >>
> >> (search for 'will render an in-line formula')
> >>
> >>
> >> AFAIK an update to commons-skin to include MathJax will not effect sites
> >> that do not contain the \( or \[ characters in plain text on site pages.
> >>
> >>
> >> I propose to:
> >>
> >> - Update commons-skin to add a MathJax script to the <head> section
> >>
> >> - Release commons-skin (last release was May 2015)
> >>
> >> - Update commons-parent:
> >>
> >> - Use the new commons-skin
> >> - Remove JaCoCo aggregate reports
> >> - Reconfigure japicmp for clean opt-in
> >>
> >>
> >> Alex
> >>
> >>
> >> [1] https://www.mathjax.org/
> >>
> >>
> >>
> >> ---------------------------------------------------------------------
> >> To unsubscribe, e-mail: [hidden email]
> >> For additional commands, e-mail: [hidden email]
> >>
> >>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

sebb-2-2
In reply to this post by Gilles Sadowski-2
On Wed, 13 Nov 2019 at 14:38, Gilles Sadowski <[hidden email]> wrote:

> Hello.
>
> Le mer. 13 nov. 2019 à 14:49, sebb <[hidden email]> a écrit :
> >
> > +1 for CP
> >
> > CP versions are entirely optional;
>
> Not "entirely".  [We already had this discussion.]
> It happened that a fix for <something> was available in a newer CP but
> that <something else> in CP broke some components (e.g. when CP
> functionality is "enhanced" based on assumptions not valid for all
> components).
> Then, that *suddenly* unsupported component has to cherry-pick and
> duplicate (in its local POM) what exists in that newer CP which it cannot
> use anymore.
>
>
The newer version was still optional.

By which I mean that it does not affect components that do not choose to
use it.
Also if a particular version is problematic, it can be ignored by all
components.

In the case above, the newer CP version was faulty: it fixed some things
and broke others.

Components that were affected by the breakages still had the choice to
continue with the previous version.
They were no worse off than before - but of course the new version did not
help them either.

> Gilles
>
> > a new version is only used if a
> > component chooses to use it.
> >
> > Is this also true of commons-skin?
> > If so, then +1
> >
> > If not, then extra care needs to be taken to ensure backwards
> compatibilty.
> > There should probably be a formal vote on the actual changes in that
> case.
> >
> >
> > On Wed, 13 Nov 2019 at 13:32, Alex Herbert <[hidden email]>
> wrote:
> >
> > > The recent release of RNG has highlighted some issues with the commons
> > > parent configuration for multi-module builds and a desirable change to
> > > commons skin.
> > >
> > > 1. [parent] JaCoCo aggregate reports are included.
> > >
> > > Aggregate reports show coverage of dependencies. Since most commons
> > > components are stand-alone this should be disabled and the report set
> > > updated to have non-aggregate reports.
> > >
> > > An example of the output is shown for the recently release BCEL:
> > >
> > > https://commons.apache.org/proper/commons-bcel/project-reports.html
> > >
> > > The aggregate report is empty.
> > >
> > >
> > > 2. [parent] japicmp does not allow clean opt-in
> > >
> > > The japicmp maven plugin is immature. If set to skip the report it
> still
> > > creates a menu entry in the site reports page. See this release of
> > > Collections:
> > >
> > >
> https://commons.apache.org/proper/commons-collections/project-reports.html
> > >
> > > The link for the japicmp report is a blank page.
> > >
> > >
> > > For collections this could be fixed by running the report.
> > >
> > > For a multi-module build using japicmp any module with no code (i.e. a
> > > pom) has this empty entry if japicmp is enabled as the skip
> > > functionality does not work.
> > >
> > >
> > > The solution is to move the reporting section from the main
> > > configuration into the japicmp profile. This allows opt-in on a
> > > per-module basis to japicmp.
> > >
> > >
> > > 3. [skin] Customisation of the site <head> section
> > >
> > > RNG uses formulas in the site documentation. Ideally this would be
> > > rendered latex using MathJax [1].
> > >
> > > This cannot be included in the site descriptor for the project as the
> > > <head> entry for each page is generated by commons-skin. This adds
> > > javascript to allow pretty print of code inside <pre "prettyprint">
> > > tags. I would like to add a javascript tag to allow inclusion of
> MathJax.
> > >
> > >
> > > I have rendered the RNG site using these changes and staged it here:
> > >
> > > mvn package site site:stage -Dcommons.release.version=1.2
> > > -DcomparisonVersion=1.2
> > >
> > > https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
> > >
> > >
> > > Top level reports page does not have japicmp:
> > >
> > >
> https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
> > >
> > > Components do:
> > >
> > >
> > >
> https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
> > >
> > > (There are also no jacoco aggregate reports.)
> > >
> > >
> > > You can view how MathJax is rendered on this page:
> > >
> > > https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
> > >
> > > (search for 'will render an in-line formula')
> > >
> > >
> > > AFAIK an update to commons-skin to include MathJax will not effect
> sites
> > > that do not contain the \( or \[ characters in plain text on site
> pages.
> > >
> > >
> > > I propose to:
> > >
> > > - Update commons-skin to add a MathJax script to the <head> section
> > >
> > > - Release commons-skin (last release was May 2015)
> > >
> > > - Update commons-parent:
> > >
> > > - Use the new commons-skin
> > > - Remove JaCoCo aggregate reports
> > > - Reconfigure japicmp for clean opt-in
> > >
> > >
> > > Alex
> > >
> > >
> > > [1] https://www.mathjax.org/
> > >
> > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> > >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Gilles Sadowski-2
Le mer. 13 nov. 2019 à 15:52, sebb <[hidden email]> a écrit :

>
> On Wed, 13 Nov 2019 at 14:38, Gilles Sadowski <[hidden email]> wrote:
>
> > Hello.
> >
> > Le mer. 13 nov. 2019 à 14:49, sebb <[hidden email]> a écrit :
> > >
> > > +1 for CP
> > >
> > > CP versions are entirely optional;
> >
> > Not "entirely".  [We already had this discussion.]
> > It happened that a fix for <something> was available in a newer CP but
> > that <something else> in CP broke some components (e.g. when CP
> > functionality is "enhanced" based on assumptions not valid for all
> > components).
> > Then, that *suddenly* unsupported component has to cherry-pick and
> > duplicate (in its local POM) what exists in that newer CP which it cannot
> > use anymore.
> >
> >
> The newer version was still optional.
>
> By which I mean that it does not affect components that do not choose to
> use it.
> Also if a particular version is problematic, it can be ignored by all
> components.
>
> In the case above, the newer CP version was faulty: it fixed some things
> and broke others.
>
> Components that were affected by the breakages still had the choice to
> continue with the previous version.

Yes, at the cost of having to fix/duplicate functionality that was delegated
to CP which now breaks that "promise".

> They were no worse off than before

It has happened, that an upgrade to a newer CP was *necessary*; for
example, to get the new version of a plugin (and this plugin had been
handled in CP).  Now, if the CP that provides the new version broke
something, then the plugin must now be handled at the component
level.  Which entails duplication, and is contrary to having a *shared*
configuration (where improvements benefit everyone).

> - but of course the new version did not
> help them either.

The point is that a CP upgrade should *not* be allowed to break
existing components that rely on the last released version.
Perhaps a Jenkins job could help assessing the impact of a using
an alternative CP (?).

Gilles

> >
> > > a new version is only used if a
> > > component chooses to use it.
> > >
> > > Is this also true of commons-skin?
> > > If so, then +1
> > >
> > > If not, then extra care needs to be taken to ensure backwards
> > compatibilty.
> > > There should probably be a formal vote on the actual changes in that
> > case.
> > >
> > >
> > > On Wed, 13 Nov 2019 at 13:32, Alex Herbert <[hidden email]>
> > wrote:
> > >
> > > > The recent release of RNG has highlighted some issues with the commons
> > > > parent configuration for multi-module builds and a desirable change to
> > > > commons skin.
> > > >
> > > > 1. [parent] JaCoCo aggregate reports are included.
> > > >
> > > > Aggregate reports show coverage of dependencies. Since most commons
> > > > components are stand-alone this should be disabled and the report set
> > > > updated to have non-aggregate reports.
> > > >
> > > > An example of the output is shown for the recently release BCEL:
> > > >
> > > > https://commons.apache.org/proper/commons-bcel/project-reports.html
> > > >
> > > > The aggregate report is empty.
> > > >
> > > >
> > > > 2. [parent] japicmp does not allow clean opt-in
> > > >
> > > > The japicmp maven plugin is immature. If set to skip the report it
> > still
> > > > creates a menu entry in the site reports page. See this release of
> > > > Collections:
> > > >
> > > >
> > https://commons.apache.org/proper/commons-collections/project-reports.html
> > > >
> > > > The link for the japicmp report is a blank page.
> > > >
> > > >
> > > > For collections this could be fixed by running the report.
> > > >
> > > > For a multi-module build using japicmp any module with no code (i.e. a
> > > > pom) has this empty entry if japicmp is enabled as the skip
> > > > functionality does not work.
> > > >
> > > >
> > > > The solution is to move the reporting section from the main
> > > > configuration into the japicmp profile. This allows opt-in on a
> > > > per-module basis to japicmp.
> > > >
> > > >
> > > > 3. [skin] Customisation of the site <head> section
> > > >
> > > > RNG uses formulas in the site documentation. Ideally this would be
> > > > rendered latex using MathJax [1].
> > > >
> > > > This cannot be included in the site descriptor for the project as the
> > > > <head> entry for each page is generated by commons-skin. This adds
> > > > javascript to allow pretty print of code inside <pre "prettyprint">
> > > > tags. I would like to add a javascript tag to allow inclusion of
> > MathJax.
> > > >
> > > >
> > > > I have rendered the RNG site using these changes and staged it here:
> > > >
> > > > mvn package site site:stage -Dcommons.release.version=1.2
> > > > -DcomparisonVersion=1.2
> > > >
> > > > https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
> > > >
> > > >
> > > > Top level reports page does not have japicmp:
> > > >
> > > >
> > https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
> > > >
> > > > Components do:
> > > >
> > > >
> > > >
> > https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
> > > >
> > > > (There are also no jacoco aggregate reports.)
> > > >
> > > >
> > > > You can view how MathJax is rendered on this page:
> > > >
> > > > https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
> > > >
> > > > (search for 'will render an in-line formula')
> > > >
> > > >
> > > > AFAIK an update to commons-skin to include MathJax will not effect
> > sites
> > > > that do not contain the \( or \[ characters in plain text on site
> > pages.
> > > >
> > > >
> > > > I propose to:
> > > >
> > > > - Update commons-skin to add a MathJax script to the <head> section
> > > >
> > > > - Release commons-skin (last release was May 2015)
> > > >
> > > > - Update commons-parent:
> > > >
> > > > - Use the new commons-skin
> > > > - Remove JaCoCo aggregate reports
> > > > - Reconfigure japicmp for clean opt-in
> > > >
> > > >
> > > > Alex
> > > >
> > > >
> > > > [1] https://www.mathjax.org/
> > > >
> > > >
> > > >
> > > > ---------------------------------------------------------------------
> > > > To unsubscribe, e-mail: [hidden email]
> > > > For additional commands, e-mail: [hidden email]
> > > >
> > > >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email]
> > For additional commands, e-mail: [hidden email]
> >
> >

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

sebb-2-2
On Wed, 13 Nov 2019 at 15:12, Gilles Sadowski <[hidden email]> wrote:

> Le mer. 13 nov. 2019 à 15:52, sebb <[hidden email]> a écrit :
> >
> > On Wed, 13 Nov 2019 at 14:38, Gilles Sadowski <[hidden email]>
> wrote:
> >
> > > Hello.
> > >
> > > Le mer. 13 nov. 2019 à 14:49, sebb <[hidden email]> a écrit :
> > > >
> > > > +1 for CP
> > > >
> > > > CP versions are entirely optional;
> > >
> > > Not "entirely".  [We already had this discussion.]
> > > It happened that a fix for <something> was available in a newer CP but
> > > that <something else> in CP broke some components (e.g. when CP
> > > functionality is "enhanced" based on assumptions not valid for all
> > > components).
> > > Then, that *suddenly* unsupported component has to cherry-pick and
> > > duplicate (in its local POM) what exists in that newer CP which it
> cannot
> > > use anymore.
> > >
> > >
> > The newer version was still optional.
> >
> > By which I mean that it does not affect components that do not choose to
> > use it.
> > Also if a particular version is problematic, it can be ignored by all
> > components.
> >
> > In the case above, the newer CP version was faulty: it fixed some things
> > and broke others.
> >
> > Components that were affected by the breakages still had the choice to
> > continue with the previous version.
>
> Yes, at the cost of having to fix/duplicate functionality that was
> delegated
> to CP which now breaks that "promise".
>
> > They were no worse off than before
>
> It has happened, that an upgrade to a newer CP was *necessary*; for
> example, to get the new version of a plugin (and this plugin had been
> handled in CP).  Now, if the CP that provides the new version broke
> something, then the plugin must now be handled at the component
> level.


Obviously it is not ideal that the new CP did not work for every component.
But it did not make the situation worse.

And the next version of the CP can hopefully fix the breakage.


> Which entails duplication, and is contrary to having a *shared*
> configuration (where improvements benefit everyone).
>
>
Of course.

> - but of course the new version did not
> > help them either.
>
> The point is that a CP upgrade should *not* be allowed to break
> existing components that rely on the last released version.
>

In which case we should formally vote on CP releases.

Ideally of course CP updates should always be improvements.
There are lots of different combinations of component setups, so it's very
difficult to ensure that changes will always be positive.

However, given that there is no need to use a particular CP version, this
is not an issue.
That's why we were able to adopt a lazy vote for the CP.

Think of a new CP version as similar to a release candidate.
If it does not work, then it is not adopted.

Perhaps a Jenkins job could help assessing the impact of a using
> an alternative CP (?).
>
>
Feel free to set one up.
I think it would be really difficult
(followups if any to a separate thread please)

Gilles

>
> > >
> > > > a new version is only used if a
> > > > component chooses to use it.
> > > >
> > > > Is this also true of commons-skin?
> > > > If so, then +1
> > > >
> > > > If not, then extra care needs to be taken to ensure backwards
> > > compatibilty.
> > > > There should probably be a formal vote on the actual changes in that
> > > case.
> > > >
> > > >
> > > > On Wed, 13 Nov 2019 at 13:32, Alex Herbert <[hidden email]
> >
> > > wrote:
> > > >
> > > > > The recent release of RNG has highlighted some issues with the
> commons
> > > > > parent configuration for multi-module builds and a desirable
> change to
> > > > > commons skin.
> > > > >
> > > > > 1. [parent] JaCoCo aggregate reports are included.
> > > > >
> > > > > Aggregate reports show coverage of dependencies. Since most commons
> > > > > components are stand-alone this should be disabled and the report
> set
> > > > > updated to have non-aggregate reports.
> > > > >
> > > > > An example of the output is shown for the recently release BCEL:
> > > > >
> > > > >
> https://commons.apache.org/proper/commons-bcel/project-reports.html
> > > > >
> > > > > The aggregate report is empty.
> > > > >
> > > > >
> > > > > 2. [parent] japicmp does not allow clean opt-in
> > > > >
> > > > > The japicmp maven plugin is immature. If set to skip the report it
> > > still
> > > > > creates a menu entry in the site reports page. See this release of
> > > > > Collections:
> > > > >
> > > > >
> > >
> https://commons.apache.org/proper/commons-collections/project-reports.html
> > > > >
> > > > > The link for the japicmp report is a blank page.
> > > > >
> > > > >
> > > > > For collections this could be fixed by running the report.
> > > > >
> > > > > For a multi-module build using japicmp any module with no code
> (i.e. a
> > > > > pom) has this empty entry if japicmp is enabled as the skip
> > > > > functionality does not work.
> > > > >
> > > > >
> > > > > The solution is to move the reporting section from the main
> > > > > configuration into the japicmp profile. This allows opt-in on a
> > > > > per-module basis to japicmp.
> > > > >
> > > > >
> > > > > 3. [skin] Customisation of the site <head> section
> > > > >
> > > > > RNG uses formulas in the site documentation. Ideally this would be
> > > > > rendered latex using MathJax [1].
> > > > >
> > > > > This cannot be included in the site descriptor for the project as
> the
> > > > > <head> entry for each page is generated by commons-skin. This adds
> > > > > javascript to allow pretty print of code inside <pre "prettyprint">
> > > > > tags. I would like to add a javascript tag to allow inclusion of
> > > MathJax.
> > > > >
> > > > >
> > > > > I have rendered the RNG site using these changes and staged it
> here:
> > > > >
> > > > > mvn package site site:stage -Dcommons.release.version=1.2
> > > > > -DcomparisonVersion=1.2
> > > > >
> > > > > https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
> > > > >
> > > > >
> > > > > Top level reports page does not have japicmp:
> > > > >
> > > > >
> > >
> https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
> > > > >
> > > > > Components do:
> > > > >
> > > > >
> > > > >
> > >
> https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
> > > > >
> > > > > (There are also no jacoco aggregate reports.)
> > > > >
> > > > >
> > > > > You can view how MathJax is rendered on this page:
> > > > >
> > > > >
> https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
> > > > >
> > > > > (search for 'will render an in-line formula')
> > > > >
> > > > >
> > > > > AFAIK an update to commons-skin to include MathJax will not effect
> > > sites
> > > > > that do not contain the \( or \[ characters in plain text on site
> > > pages.
> > > > >
> > > > >
> > > > > I propose to:
> > > > >
> > > > > - Update commons-skin to add a MathJax script to the <head> section
> > > > >
> > > > > - Release commons-skin (last release was May 2015)
> > > > >
> > > > > - Update commons-parent:
> > > > >
> > > > > - Use the new commons-skin
> > > > > - Remove JaCoCo aggregate reports
> > > > > - Reconfigure japicmp for clean opt-in
> > > > >
> > > > >
> > > > > Alex
> > > > >
> > > > >
> > > > > [1] https://www.mathjax.org/
> > > > >
> > > > >
> > > > >
> > > > >
> ---------------------------------------------------------------------
> > > > > To unsubscribe, e-mail: [hidden email]
> > > > > For additional commands, e-mail: [hidden email]
> > > > >
> > > > >
> > >
> > > ---------------------------------------------------------------------
> > > To unsubscribe, e-mail: [hidden email]
> > > For additional commands, e-mail: [hidden email]
> > >
> > >
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Gilles Sadowski-2
Le mer. 13 nov. 2019 à 17:03, sebb <[hidden email]> a écrit :

>
> On Wed, 13 Nov 2019 at 15:12, Gilles Sadowski <[hidden email]> wrote:
>
> > Le mer. 13 nov. 2019 à 15:52, sebb <[hidden email]> a écrit :
> > >
> > > On Wed, 13 Nov 2019 at 14:38, Gilles Sadowski <[hidden email]>
> > wrote:
> > >
> > > > Hello.
> > > >
> > > > Le mer. 13 nov. 2019 à 14:49, sebb <[hidden email]> a écrit :
> > > > >
> > > > > +1 for CP
> > > > >
> > > > > CP versions are entirely optional;
> > > >
> > > > Not "entirely".  [We already had this discussion.]
> > > > It happened that a fix for <something> was available in a newer CP but
> > > > that <something else> in CP broke some components (e.g. when CP
> > > > functionality is "enhanced" based on assumptions not valid for all
> > > > components).
> > > > Then, that *suddenly* unsupported component has to cherry-pick and
> > > > duplicate (in its local POM) what exists in that newer CP which it
> > cannot
> > > > use anymore.
> > > >
> > > >
> > > The newer version was still optional.
> > >
> > > By which I mean that it does not affect components that do not choose to
> > > use it.
> > > Also if a particular version is problematic, it can be ignored by all
> > > components.
> > >
> > > In the case above, the newer CP version was faulty: it fixed some things
> > > and broke others.
> > >
> > > Components that were affected by the breakages still had the choice to
> > > continue with the previous version.
> >
> > Yes, at the cost of having to fix/duplicate functionality that was
> > delegated
> > to CP which now breaks that "promise".
> >
> > > They were no worse off than before
> >
> > It has happened, that an upgrade to a newer CP was *necessary*; for
> > example, to get the new version of a plugin (and this plugin had been
> > handled in CP).  Now, if the CP that provides the new version broke
> > something, then the plugin must now be handled at the component
> > level.
>
>
> Obviously it is not ideal that the new CP did not work for every component.
> But it did not make the situation worse.

It did (because of the added work due to the necessity to diverge
from CP, as examplified in the provious post).

> And the next version of the CP can hopefully fix the breakage.

Or not, if the component continues to diverge until someone
wonders about the duplication.

> > Which entails duplication, and is contrary to having a *shared*
> > configuration (where improvements benefit everyone).
> >
> >
> Of course.

Glad we agree on this.

> > - but of course the new version did not
> > > help them either.
> >
> > The point is that a CP upgrade should *not* be allowed to break
> > existing components that rely on the last released version.
> >
>
> In which case we should formally vote on CP releases.

Only a veto (based on the technical reason "breaks the build")
would work.

>
> Ideally of course CP updates should always be improvements.
> There are lots of different combinations of component setups, so it's very
> difficult to ensure that changes will always be positive.

Sure.
But I still think that it when a issue arises, it should not
be dismissed with the "CP is optional" argument.

> However, given that there is no need to use a particular CP version, this
> is not an issue.
> That's why we were able to adopt a lazy vote for the CP.
>
> Think of a new CP version as similar to a release candidate.
> If it does not work, then it is not adopted.

Fine (cf. above).

> Perhaps a Jenkins job could help assessing the impact of a using
> > an alternative CP (?).
> >
> >
> Feel free to set one up.
> I think it would be really difficult

Perhaps then: for each component who fears breakage, create a
Jenkins project that track changes to CP (?).

> (followups if any to a separate thread please)

Well, it's all theoretical again, as for now, CP is fine; so, low
priority.  Hopefully, further improvement will not cut corners
(e.g. assuming non-modular components).

Regards,
Gilles

>>> [...]

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Alex Herbert

>
>>>> […]

I had a bit more time to investigate why I could not inject MathJax into the header of commons-skin using the site descriptor.

As of maven site plugin 3.5 any injected XHTML has to be escaped using CDATA XML notation [1].

Commons-skin is based on a version of Apache Maven Fluido Skin (1.3.0 - Aug, 2012). The current Fluido skin is 1.8 (Aug, 2019) [2].

Thus commons-skin predates maven site plugin 3.5 (Feb, 2016) and cannot handle the CDATA escaped XML.

If I change the skin in my site descriptor to:

    <groupId>org.apache.maven.skins</groupId>
    <artifactId>maven-default-skin</artifactId>
    <version>1.3</version>

    <groupId>org.apache.maven.skins</groupId>
    <artifactId>maven-fluido-skin</artifactId>
    <version>1.8</version>

Then the XHTML from the <head> tag is rendered.

In short the commons-skin should be updated. How much it was modified from the Fluido Skin 1.3.0 I do not know. Thus I cannot say what the best course of modification is. Start with a default skin and customise it. Or try and fix the current skin we have.

The Fluido skin supports code highlighting with prettyprint which is a nice feature of the current commons-skin.

I think I will proceed with modifying commons parent with the fixes for Jacoco and japicmp.

A new commons-skin can be investigate separately as it can be used by projects without having to update cp.


[1] https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Inject_xhtml_into_.3Chead.3E <https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Inject_xhtml_into_.3Chead.3E>

[2] https://maven.apache.org/skins/maven-fluido-skin/ <https://maven.apache.org/skins/maven-fluido-skin/>


>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email] <mailto:[hidden email]>
> For additional commands, e-mail: [hidden email] <mailto:[hidden email]>
Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

sebb-2-2
On Wed, 13 Nov 2019 at 22:59, Alex Herbert <[hidden email]> wrote:

>
> >
> >>>> […]
>
> I had a bit more time to investigate why I could not inject MathJax into
> the header of commons-skin using the site descriptor.
>
> As of maven site plugin 3.5 any injected XHTML has to be escaped using
> CDATA XML notation [1].
>
> Commons-skin is based on a version of Apache Maven Fluido Skin (1.3.0 -
> Aug, 2012). The current Fluido skin is 1.8 (Aug, 2019) [2].
>
> Thus commons-skin predates maven site plugin 3.5 (Feb, 2016) and cannot
> handle the CDATA escaped XML.
>
> If I change the skin in my site descriptor to:
>
>     <groupId>org.apache.maven.skins</groupId>
>     <artifactId>maven-default-skin</artifactId>
>     <version>1.3</version>
>
>     <groupId>org.apache.maven.skins</groupId>
>     <artifactId>maven-fluido-skin</artifactId>
>     <version>1.8</version>
>
> Then the XHTML from the <head> tag is rendered.
>
> In short the commons-skin should be updated. How much it was modified from
> the Fluido Skin 1.3.0 I do not know.


Does a diff between the two not show anything useful?


> Thus I cannot say what the best course of modification is. Start with a
> default skin and customise it. Or try and fix the current skin we have.
>
> The Fluido skin supports code highlighting with prettyprint which is a
> nice feature of the current commons-skin.
>
> I think I will proceed with modifying commons parent with the fixes for
> Jacoco and japicmp.
>
> A new commons-skin can be investigate separately as it can be used by
> projects without having to update cp.
>
>
> [1]
> https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Inject_xhtml_into_.3Chead.3E
> <
> https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Inject_xhtml_into_.3Chead.3E
> >
>
> [2] https://maven.apache.org/skins/maven-fluido-skin/ <
> https://maven.apache.org/skins/maven-fluido-skin/>
>
>
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: [hidden email] <mailto:
> [hidden email]>
> > For additional commands, e-mail: [hidden email] <mailto:
> [hidden email]>
>
Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Alex Herbert


> On 14 Nov 2019, at 23:14, sebb <[hidden email]> wrote:
>
> On Wed, 13 Nov 2019 at 22:59, Alex Herbert <[hidden email] <mailto:[hidden email]>> wrote:
>
>>
>>>
>>>>>> […]
>>
>> I had a bit more time to investigate why I could not inject MathJax into
>> the header of commons-skin using the site descriptor.
>>
>> As of maven site plugin 3.5 any injected XHTML has to be escaped using
>> CDATA XML notation [1].
>>
>> Commons-skin is based on a version of Apache Maven Fluido Skin (1.3.0 -
>> Aug, 2012). The current Fluido skin is 1.8 (Aug, 2019) [2].
>>
>> Thus commons-skin predates maven site plugin 3.5 (Feb, 2016) and cannot
>> handle the CDATA escaped XML.
>>
>> If I change the skin in my site descriptor to:
>>
>>    <groupId>org.apache.maven.skins</groupId>
>>    <artifactId>maven-default-skin</artifactId>
>>    <version>1.3</version>
>>
>>    <groupId>org.apache.maven.skins</groupId>
>>    <artifactId>maven-fluido-skin</artifactId>
>>    <version>1.8</version>
>>
>> Then the XHTML from the <head> tag is rendered.
>>
>> In short the commons-skin should be updated. How much it was modified from
>> the Fluido Skin 1.3.0 I do not know.
>
>
> Does a diff between the two not show anything useful?

I’ve been looking. A lot of the functions have been deleted. Some of the text matches exactly. But some is not like for like identical. The commons skin was done in Feb 2014. I was using the Fluido from 1.3.0 (Aug 2012). I will look at 1.3.1 (Dec 2013) as well to see if that was the base instead.

The skin header has some comments as to what it is doing that is specific for commons. This mostly seems to be done at the top and then the footer. So I hope to apply the same mods to a new skin. The commons style sheets should remain the same and they would be the main source of preserving the look of the site.

The velocity template is concerned with menu and navigation layout. I will do some work on a forked branch of commons-skin and see how far I get.

The key to a new skin adaption is to use small commits starting from the original source to ease traceability. The previous skin was deposited in a single large diff with the customisations in place. This is not helping me understand the customisations.


>
>
>> Thus I cannot say what the best course of modification is. Start with a
>> default skin and customise it. Or try and fix the current skin we have.
>>
>> The Fluido skin supports code highlighting with prettyprint which is a
>> nice feature of the current commons-skin.
>>
>> I think I will proceed with modifying commons parent with the fixes for
>> Jacoco and japicmp.
>>
>> A new commons-skin can be investigate separately as it can be used by
>> projects without having to update cp.
>>
>>
>> [1]
>> https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Inject_xhtml_into_.3Chead.3E
>> <
>> https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Inject_xhtml_into_.3Chead.3E <https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Inject_xhtml_into_.3Chead.3E>
>>>
>>
>> [2] https://maven.apache.org/skins/maven-fluido-skin/ <https://maven.apache.org/skins/maven-fluido-skin/> <
>> https://maven.apache.org/skins/maven-fluido-skin/ <https://maven.apache.org/skins/maven-fluido-skin/>>
>>
>>
>>>
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: [hidden email] <mailto:[hidden email]> <mailto:
>> [hidden email] <mailto:[hidden email]>>
>>> For additional commands, e-mail: [hidden email] <mailto:[hidden email]> <mailto:
>> [hidden email] <mailto:[hidden email]>>

Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Bruno P. Kinoshita-2
In reply to this post by Alex Herbert
 Hi Alex,
>This cannot be included in the site descriptor for the project as the
><head> entry for each page is generated by commons-skin. This adds
>javascript to allow pretty print of code inside <pre "prettyprint">
>tags. I would like to add a javascript tag to allow inclusion of MathJax.
What about loading per page in RNG?
> - Update commons-skin to add a MathJax script to the <head> section
Not sure if that's the best option. That would be loaded for every component I assume. In case a new version was needed we would have to release a new version of commons-skin too I guess?
I had a look at commons-parent and commons-skin. Looks like commons-parent defines the body.head, as well as body.custom.footer. Which are places that we could load the script.
I think only the menu can be extended in the child projects (ASF components), so we have no way of using those elements to customize RNG (as you pointed above).

Here's one way I found that works, where a component loads an extra JS script. In this example I used commons-text (already had cloned in my notebook) and loaded mathjax to render an equation from their tutorial.
https://github.com/apache/commons-text/compare/master...kinow:mathjax-site?expand=1
If that's not good, then I guess we would have to either change maven-site-plugin to allow scripts to be loaded, or go with your original suggestion.

Didn't have time to look at the other issues with JaCoCo and jacimp, sorry.

Hope that helpsBruno

ps: there is a CDATA element to prevent the maven-site-plugin from filtering the JS script... maybe a bit too hacky?

    On Thursday, 14 November 2019, 2:32:56 am NZDT, Alex Herbert <[hidden email]> wrote:  
 
 The recent release of RNG has highlighted some issues with the commons
parent configuration for multi-module builds and a desirable change to
commons skin.

1. [parent] JaCoCo aggregate reports are included.

Aggregate reports show coverage of dependencies. Since most commons
components are stand-alone this should be disabled and the report set
updated to have non-aggregate reports.

An example of the output is shown for the recently release BCEL:

https://commons.apache.org/proper/commons-bcel/project-reports.html

The aggregate report is empty.


2. [parent] japicmp does not allow clean opt-in

The japicmp maven plugin is immature. If set to skip the report it still
creates a menu entry in the site reports page. See this release of
Collections:

https://commons.apache.org/proper/commons-collections/project-reports.html

The link for the japicmp report is a blank page.


For collections this could be fixed by running the report.

For a multi-module build using japicmp any module with no code (i.e. a
pom) has this empty entry if japicmp is enabled as the skip
functionality does not work.


The solution is to move the reporting section from the main
configuration into the japicmp profile. This allows opt-in on a
per-module basis to japicmp.


3. [skin] Customisation of the site <head> section

RNG uses formulas in the site documentation. Ideally this would be
rendered latex using MathJax [1].

This cannot be included in the site descriptor for the project as the
<head> entry for each page is generated by commons-skin. This adds
javascript to allow pretty print of code inside <pre "prettyprint">
tags. I would like to add a javascript tag to allow inclusion of MathJax.


I have rendered the RNG site using these changes and staged it here:

mvn package site site:stage -Dcommons.release.version=1.2
-DcomparisonVersion=1.2

https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html


Top level reports page does not have japicmp:

https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html

Components do:

https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html

(There are also no jacoco aggregate reports.)


You can view how MathJax is rendered on this page:

https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html

(search for 'will render an in-line formula')


AFAIK an update to commons-skin to include MathJax will not effect sites
that do not contain the \( or \[ characters in plain text on site pages.


I propose to:

- Update commons-skin to add a MathJax script to the <head> section

- Release commons-skin (last release was May 2015)

- Update commons-parent:

- Use the new commons-skin
- Remove JaCoCo aggregate reports
- Reconfigure japicmp for clean opt-in


Alex


[1] https://www.mathjax.org/



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

 
Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Alex Herbert

On 15/11/2019 05:02, Bruno P. Kinoshita wrote:
>   Hi Alex,
>> This cannot be included in the site descriptor for the project as the
>> <head> entry for each page is generated by commons-skin. This adds
>> javascript to allow pretty print of code inside <pre "prettyprint">
>> tags. I would like to add a javascript tag to allow inclusion of MathJax.
> What about loading per page in RNG?

Thanks for the suggestion.

The main user guide is written in APT [1].

This cannot include html tags. I could covert APT to XDOC but there are
more issues with the commons-skin that should be fixed.

[1] https://maven.apache.org/doxia/references/apt-format.html

>> - Update commons-skin to add a MathJax script to the <head> section
> Not sure if that's the best option. That would be loaded for every component I assume. In case a new version was needed we would have to release a new version of commons-skin too I guess?
> I had a look at commons-parent and commons-skin. Looks like commons-parent defines the body.head, as well as body.custom.footer. Which are places that we could load the script.
> I think only the menu can be extended in the child projects (ASF components), so we have no way of using those elements to customize RNG (as you pointed above).

The <head> and <custom> can be overwritten from the inherited site.xml
in commons parent. So this is OK if they worked. Unfortunately they do
not for the reasons outlined below.

I can include MathJax if I use a different skin that supports XHTML
injection into <head>. It just does not work with commons-skin.

This is because we now use maven site plugin 3.5+ which has to contain
injected XHTML <script> tags in CDATA for the <head> section.

The commons-skin was pre maven 3.5 so does not support this.

This is the crux of my problem where I cannot inject the MathJax script
into the <head>. If I remove the <![CDATA[ I get an error:

[ERROR] Failed to execute goal
org.apache.maven.plugins:maven-site-plugin:3.8.2:site (default-cli) on
project commons-rng-parent: SiteToolException: Error parsing site
descriptor: expected = after attribute name (position: TEXT seen
...>\n      <script type="text/javascript" id="MathJax-script" async
s... @37:65) -> [Help 1]

It is not allowed with the current site plugin.

I've noticed that the following footer defined in commons-parent is also
using a CDATA tag:

   <!-- inherited -->
   <custom>
     <![CDATA[
     <!-- @project.name@ will be replaced by the template; if we used
${project.name} it would pick up Commons Parent -->
     <footer>
       <div class="center">
       Apache Commons, Apache @project.name@, Apache, the Apache feather
logo, and the Apache Commons project logos are trademarks of The Apache
Software Foundation.
       All other marks mentioned may be trademarks or registered
trademarks of their respective owners.
       </div>
     </footer>
     ]]>
   </custom>

This does not work and all of our site pages do not have the copyright
notice on them.

It is not actually needed as the custom tag is ignored by the site
plugin [2]. You can put what you want in there.

If I update commons parent to fix this (drop the <![CDATA[ tag in the
custom footer) then the copyright notice does not work correctly as the
macros are outdated. The velocity macro in commons-skin is using:

$StringUtils.replace()

This does not appear to exist in velocity 1.7 [3]. So the copyright
footer is then rendered as:

Apache Commons, Apache Apache Commons RNG, Apache, the Apache feather
logo, and the Apache Commons project logos are trademarks of The Apache
Software Foundation. All other marks mentioned may be trademarks or
registered trademarks of their respective owners.

With the duplicate 'Apache Apache'.

So to render the copyright footer will require an update to commons-skin
and a fix for commons parent to drop the ![CDATA[ tag in the custom footer.


My other issue is that the pretty print functionality for code
highlighting does not work when APT pages are rendered into <pre> tags
as the classes used are now different to those targetted by the style
sheets in commons skin.

This has been fixed in the fluido skin 1.8. So I am working on bring the
changes made to fluido 1.3 for the commons skin to fluido 1.8.


[2]
https://maven.apache.org/plugins/maven-site-plugin/examples/sitedescriptor.html#Custom_content

[3]
https://velocity.apache.org/engine/1.7/apidocs/org/apache/velocity/util/StringUtils.html


> Here's one way I found that works, where a component loads an extra JS script. In this example I used commons-text (already had cloned in my notebook) and loaded mathjax to render an equation from their tutorial.
> https://github.com/apache/commons-text/compare/master...kinow:mathjax-site?expand=1
> If that's not good, then I guess we would have to either change maven-site-plugin to allow scripts to be loaded, or go with your original suggestion.
>
> Didn't have time to look at the other issues with JaCoCo and jacimp, sorry.
Those issue I have already fixed with updates to commons parent.

>
> Hope that helpsBruno
>
> ps: there is a CDATA element to prevent the maven-site-plugin from filtering the JS script... maybe a bit too hacky?
>
>      On Thursday, 14 November 2019, 2:32:56 am NZDT, Alex Herbert <[hidden email]> wrote:
>  
>   The recent release of RNG has highlighted some issues with the commons
> parent configuration for multi-module builds and a desirable change to
> commons skin.
>
> 1. [parent] JaCoCo aggregate reports are included.
>
> Aggregate reports show coverage of dependencies. Since most commons
> components are stand-alone this should be disabled and the report set
> updated to have non-aggregate reports.
>
> An example of the output is shown for the recently release BCEL:
>
> https://commons.apache.org/proper/commons-bcel/project-reports.html
>
> The aggregate report is empty.
>
>
> 2. [parent] japicmp does not allow clean opt-in
>
> The japicmp maven plugin is immature. If set to skip the report it still
> creates a menu entry in the site reports page. See this release of
> Collections:
>
> https://commons.apache.org/proper/commons-collections/project-reports.html
>
> The link for the japicmp report is a blank page.
>
>
> For collections this could be fixed by running the report.
>
> For a multi-module build using japicmp any module with no code (i.e. a
> pom) has this empty entry if japicmp is enabled as the skip
> functionality does not work.
>
>
> The solution is to move the reporting section from the main
> configuration into the japicmp profile. This allows opt-in on a
> per-module basis to japicmp.
>
>
> 3. [skin] Customisation of the site <head> section
>
> RNG uses formulas in the site documentation. Ideally this would be
> rendered latex using MathJax [1].
>
> This cannot be included in the site descriptor for the project as the
> <head> entry for each page is generated by commons-skin. This adds
> javascript to allow pretty print of code inside <pre "prettyprint">
> tags. I would like to add a javascript tag to allow inclusion of MathJax.
>
>
> I have rendered the RNG site using these changes and staged it here:
>
> mvn package site site:stage -Dcommons.release.version=1.2
> -DcomparisonVersion=1.2
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/index.html
>
>
> Top level reports page does not have japicmp:
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/project-reports.html
>
> Components do:
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/commons-rng-simple/project-reports.html
>
> (There are also no jacoco aggregate reports.)
>
>
> You can view how MathJax is rendered on this page:
>
> https://home.apache.org/~aherbert/commons-rng-1.4-site/developers.html
>
> (search for 'will render an in-line formula')
>
>
> AFAIK an update to commons-skin to include MathJax will not effect sites
> that do not contain the \( or \[ characters in plain text on site pages.
>
>
> I propose to:
>
> - Update commons-skin to add a MathJax script to the <head> section
>
> - Release commons-skin (last release was May 2015)
>
> - Update commons-parent:
>
> - Use the new commons-skin
> - Remove JaCoCo aggregate reports
> - Reconfigure japicmp for clean opt-in
>
>
> Alex
>
>
> [1] https://www.mathjax.org/
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>    

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Alex Herbert


> On 15 Nov 2019, at 14:40, Alex Herbert <[hidden email]> wrote:
>
> […]

I have finished testing different solutions to the site rendering of RNG to included MathJax and render the code using prettyprint.

I tried a full upgrade to commons-skin to base it on Fluido 1.8. This changed a lot of element class names such that the style sheet work was too time consuming. I fixed it for RNG but there is more to the commons-skin customisation that I have not yet tested. It allowed dynamic rendering of the left menu width. This is fixed at 250px for all published commons sites. Thus the new and old skins looked different enough that it would require more comprehensive testing.

In this process I now understand where the problems are with common-skin:

1. <head> processing

The commons-skin uses the default <head> processing from Fluido 1.3. This was written pre maven site plugin 3.5 where <head> element allows XHTML. Now this has to be escaped. The velocity template code thus does not work. This requires a change to the velocity template to use the rendering engine to run the velocity engine on the injected XHTML. This is what is does in Fluido 1.8.

Given the current commons-skin functionality for <head> is broken this is a simple fix that should be made.


2. The footer

The footer for all commons pages in defined in the site.xml in commons parent. This is within a CDATA escape tag. This should not be done.

The commons-skin template does not work with the CDATA escaped footer. Thus more recent site for commons components have no copyright footer.

If you remove the CDATA escape tag from commons-parent then you see that the macro for the footer is not working correctly and the footer although now included has some incorrect text (duplicate 'Apache Apache’ in the text).

The fix is simple and should be applied to the current commons-skin


3. Menu icons

I noticed the right arrow menu icon has a white background. The icon is 7x7 pixels and the background is light grey so it is hard to notice. The down arrow has a transparent background. So this should be fixed.


4. Pretty print of source code

The site.js in commons skin has a workaround for the fact that any <pre> tag are wrapped in a <div> tag.

Thus in XDOC:

<source class=“prettyprint”>

Turns into

<div class=“prettyprint”><pre>

This is handled in site.js by javascript that moves the prettyprint class from the <div> tag to the <pre> tag.


The issue with commons RNG is the use of APT to write the user guide.

Thus in APT:

*-------------
String s = “”;
*-------------

Turns into

<div class=“source”><pre>


The <pre> tag is not pretty printed.


Fluido 1.8 handles the second case with this replacement in the body of the document:

$bodyContent.replaceAll( "<div class=${esc.q}source${esc.q}>(\r?\n)?<pre>", "<div class=${esc.q}source${esc.q}><pre class=${esc.q}$sourceStyle${esc.q}>" )

Replacing:
<div class=“source”><pre>

With
<div class=“source”><pre class="prettyprint">



Proposal:

I propose to fix items 1, 2, 3 in commons-skin. These are all bugs where the intended functionality is not working.

Item 4 would be a new change to increase the coverage of prettyprint to more <pre> tags. This should have a vote if it should be opt out or opt in.

If the regex replacement is done in the site.vm velocity template (like Fluido 1.8) then opt out/in could be done by passing a properties to the commons-skin plugin using the site.xml:

           <custom>
             <commonsSkin>
               <prettyPrintSourcePreTags>true</prettyPrintSourcePreTags>
               <sourceLineNumbersEnabled>false</sourceLineNumbersEnabled>
             </commonsSkin >
           </custom>

The second property allows the class “linenums” to be added to the tag as well for different prettyprint rendering. This option idea is copied from Fluido 1.8 [1].

If I make this change then all existing functionality of commons-skin should be restored. Commons RNG would be able to opt-in configure the pretty print for the APT written site documentation.

In my local git repos for commons only commons-math uses APT documentation. In this document there are no source snippets. So it may be OK to make this opt out functionality that is on by default. But I am missing a lot of the projects and some may be effected.

I have confirmed that if I put <prettyPrintSourcePreTags>true</prettyPrintSourcePreTags> in commons-parent site.xml and then <prettyPrintSourcePreTags>false</prettyPrintSourcePreTags> in commons-rng then the functionality is turned off. So opt out would work for any projects that upgrade and have an issue with the prettyprint rendering.

I would vote for fixing the bugs in commons-skin and making the new functionality opt-out via properties in the site.xml. The skin can then be released and then used with the most recent commons-parent. This would fix the problem with the CDATA wrapper footer in the site.xml.

Alex

[1] https://maven.apache.org/skins/maven-fluido-skin/#SourceLineNumbers <https://maven.apache.org/skins/maven-fluido-skin/#SourceLineNumbers>



Reply | Threaded
Open this post in threaded view
|

Re: [ALL] Updates to commons-parent and commons-skin

Bruno P. Kinoshita-2
 Sounds good to me, +1
And thanks for the detailed report on your investigation Alex!
Bruno

    On Monday, 18 November 2019, 3:08:49 am NZDT, Alex Herbert <[hidden email]> wrote:  
 
 

> On 15 Nov 2019, at 14:40, Alex Herbert <[hidden email]> wrote:
>
> […]

I have finished testing different solutions to the site rendering of RNG to included MathJax and render the code using prettyprint.

I tried a full upgrade to commons-skin to base it on Fluido 1.8. This changed a lot of element class names such that the style sheet work was too time consuming. I fixed it for RNG but there is more to the commons-skin customisation that I have not yet tested. It allowed dynamic rendering of the left menu width. This is fixed at 250px for all published commons sites. Thus the new and old skins looked different enough that it would require more comprehensive testing.

In this process I now understand where the problems are with common-skin:

1. <head> processing

The commons-skin uses the default <head> processing from Fluido 1.3. This was written pre maven site plugin 3.5 where <head> element allows XHTML. Now this has to be escaped. The velocity template code thus does not work. This requires a change to the velocity template to use the rendering engine to run the velocity engine on the injected XHTML. This is what is does in Fluido 1.8.

Given the current commons-skin functionality for <head> is broken this is a simple fix that should be made.


2. The footer

The footer for all commons pages in defined in the site.xml in commons parent. This is within a CDATA escape tag. This should not be done.

The commons-skin template does not work with the CDATA escaped footer. Thus more recent site for commons components have no copyright footer.

If you remove the CDATA escape tag from commons-parent then you see that the macro for the footer is not working correctly and the footer although now included has some incorrect text (duplicate 'Apache Apache’ in the text).

The fix is simple and should be applied to the current commons-skin


3. Menu icons

I noticed the right arrow menu icon has a white background. The icon is 7x7 pixels and the background is light grey so it is hard to notice. The down arrow has a transparent background. So this should be fixed.


4. Pretty print of source code

The site.js in commons skin has a workaround for the fact that any <pre> tag are wrapped in a <div> tag.

Thus in XDOC:

<source class=“prettyprint”>

Turns into

<div class=“prettyprint”><pre>

This is handled in site.js by javascript that moves the prettyprint class from the <div> tag to the <pre> tag.


The issue with commons RNG is the use of APT to write the user guide.

Thus in APT:

*-------------
String s = “”;
*-------------

Turns into

<div class=“source”><pre>


The <pre> tag is not pretty printed.


Fluido 1.8 handles the second case with this replacement in the body of the document:

$bodyContent.replaceAll( "<div class=${esc.q}source${esc.q}>(\r?\n)?<pre>", "<div class=${esc.q}source${esc.q}><pre class=${esc.q}$sourceStyle${esc.q}>" )

Replacing:
<div class=“source”><pre>

With
<div class=“source”><pre class="prettyprint">



Proposal:

I propose to fix items 1, 2, 3 in commons-skin. These are all bugs where the intended functionality is not working.

Item 4 would be a new change to increase the coverage of prettyprint to more <pre> tags. This should have a vote if it should be opt out or opt in.

If the regex replacement is done in the site.vm velocity template (like Fluido 1.8) then opt out/in could be done by passing a properties to the commons-skin plugin using the site.xml:

      <custom>
        <commonsSkin>
          <prettyPrintSourcePreTags>true</prettyPrintSourcePreTags>
              <sourceLineNumbersEnabled>false</sourceLineNumbersEnabled>
        </commonsSkin >
      </custom>

The second property allows the class “linenums” to be added to the tag as well for different prettyprint rendering. This option idea is copied from Fluido 1.8 [1].

If I make this change then all existing functionality of commons-skin should be restored. Commons RNG would be able to opt-in configure the pretty print for the APT written site documentation.

In my local git repos for commons only commons-math uses APT documentation. In this document there are no source snippets. So it may be OK to make this opt out functionality that is on by default. But I am missing a lot of the projects and some may be effected.

I have confirmed that if I put <prettyPrintSourcePreTags>true</prettyPrintSourcePreTags> in commons-parent site.xml and then <prettyPrintSourcePreTags>false</prettyPrintSourcePreTags> in commons-rng then the functionality is turned off. So opt out would work for any projects that upgrade and have an issue with the prettyprint rendering.

I would vote for fixing the bugs in commons-skin and making the new functionality opt-out via properties in the site.xml. The skin can then be released and then used with the most recent commons-parent. This would fix the problem with the CDATA wrapper footer in the site.xml.

Alex

[1] https://maven.apache.org/skins/maven-fluido-skin/#SourceLineNumbers <https://maven.apache.org/skins/maven-fluido-skin/#SourceLineNumbers>