I investigated the patch-sitemap.js question a bit. Using my `issue-16854-jsonpath-options` camel-website branch, I built the site twice, with site:url set to `https://camel.apache.org` and set to `/`. I didn’t look at every page, but diffing the generated sites seems to consistently show two differences between the results:
- with `https://camel.apache.org` a head <link> element is included such as <link rel="canonical" href="https://camel.apache.org/components/latest/ironmq-component.html";> It’s omitted as expected with `/`. My understanding is that this needs to be an absolute URI and that it’s function is to help search engines. However, if we don’t want it, it’s trivial to modify the UI to not generate it. - with `https://camel.apache.org` a footer micro data script is plausible rather than meaningless (the `url:` entry): <script type='application/ld+json'> { "@context": "http://schema.org";, "@type": "Organization", "name": "Apache Camel", "url": "https://camel.apache.org";, "sameAs": [ "https://twitter.com/ApacheCamel"; ], "logo": "../../_/img/logo-d.svg", "description": "Apache Camel ™ is a versatile open-source integration framework based on known Enterprise Integration Patterns. Camel empowers you to define routing and mediation rules in a variety of domain-specific languages, including a Java-based Fluent API, Spring or Blueprint XML Configuration files, and a Scala DSL." } versus <script type='application/ld+json'> { "@context": "http://schema.org";, "@type": "Organization", "name": "Apache Camel", "url": "/", "sameAs": [ "https://twitter.com/ApacheCamel"; ], "logo": "../../_/img/logo-d.svg", "description": "Apache Camel ™ is a versatile open-source integration framework based on known Enterprise Integration Patterns. Camel empowers you to define routing and mediation rules in a variety of domain-specific languages, including a Java-based Fluent API, Spring or Blueprint XML Configuration files, and a Scala DSL." } </script> In both cases I’d expect that to be usable the logo should be an absolute URI? I note that the next bit of micro data, BreadcrumbList, is always generated with absolute URIs with https://camel.apache.org. Shouldn’t this be generated from the site:url? My conclusions are: - There is no need for patch-sitemap.js and that the site needs to be generated with the correct site:url. - If inclusion of the <link> element causes a problem it can be removed from the UI. - The Organization micro data needs it’s logo URL fixed to be absolute based on site:url - The BreadcrumbList micro data needs to be generated based on site:url. Have I missed something? David Jencks > On Sep 7, 2021, at 8:22 AM, David Jencks <david.a.jen...@gmail.com> wrote: > > Hi Zoran, > > I just pushed some more fixes, I hope this isn’t making reviewing too hard. > > Starting far back in the email chain is probably pretty confusing, since I or > others have fixed a lot of the problems noted. I’ve tried to summarize all > the current problems in each email: I think I should have also noted which > ones got fixed. > > Some comments inline. > >> On Sep 7, 2021, at 4:42 AM, Zoran Regvart <zo...@regvart.com >> <mailto:zo...@regvart.com>> wrote: >> >> Hi David, >> working through the changes and emails, some more comments here... >> >> On Fri, Sep 3, 2021 at 8:21 AM David Jencks <david.a.jen...@gmail.com >> <mailto:david.a.jen...@gmail.com>> wrote: >>> comments: >>> >>> - I removed the patch-sitemap.js use. I can’t figure out why this is >>> needed. If it really is needed, it should be done with a pipeline >>> extension. >> >> I think we'll need to restore this, otherwise we'll lose the sitemap >> generation which in turn leaves us with outdated links on the search >> engines, and as we change the links when we add or remove new >> documentation versions this becomes an issue. >> > > I’m rather confused by this since my experience and Dan’s repeated statements > are that Antora always generates relative links within the site. It’s > possible to write the UI to generate either absolute or relative links, but > that’s not something Antora is doing itself. Getting Antora to generate > absolute links in page bodies (not the UI) requires major surgery :-) So, > I’ll compare generating with absolute and ‘/‘ siteUrl and come back with more > definite info. > >>> - yarn workspace —topological-dev is not building the UI before it’s used. >>> I can’t figure out why; it is for other PRs. I replaced this by just >>> calling the UI build directly. >> >> That's odd, I did not notice this issue before, I'll try to reproduce >> it and see if there is something we can do about this, this should be >> one of the benefits in using workspaces. > > It doesn’t seem to afflict other people’s PRs :-), but I have not yet had any > build use the correct order with —topological-dev. One run showing the > problem is > https://github.com/apache/camel-website/runs/3453859096?check_suite_focus=true > > <https://github.com/apache/camel-website/runs/3453859096?check_suite_focus=true>. > I wonder if it’s related to checking in the wrong .yarn contents: I think > in this particular run I hadn’t checked in any .yarn changes, but later I > have been checking in such changes and still get the wrong order. >> >>> - If nothing else is using the model building in UpdateReadmeMojo it can >>> probably be simplified further. >> >> From what I understand we only need the JSON files generated now, >> perhaps the we don't need the UpdateReadmeMojo at all any more? > > Perhaps. It still generates the doc header. If Allow asciidoctor extensions > when processing nav files and headers > <https://gitlab.com/antora/antora/-/issues/592> were implemented, the header > generation could be done by the jsonpath extension, but I’m not sure how much > of an improvement that would be. In any case, UpdateReadmeMojo is now more > of a sanity checker than content generator :-). >> >>> - The camel-website node dependencies have changed dramatically. It looks >>> like the yarn cache is checked into git; I haven’t checked in the changes. >>> Should I? >> > > Later on I started checking in .yarn, but I’m really not sure if I’m getting > appropriate content. I think that previously the UI was not in the main > .yarn, but at least sometimes I get it, and it’s > 50 MB and I get a warning. > > Note that I have 2 camel-website PRs: the “antora 3” one is the one that > should be merged, and after the camel jsonpath-options PR is merged it will > build with all the new stuff. The other camel-website PR is just to build > the jsonpath-options branch. >> >>> known problems: >>> >>> - When the first row contains a description with a list, the list isn’t >>> rendered properly. cf. >>> https://pr-614--camel.netlify.app/components/latest/activemq-component.html#_path_parameters_2_parameters >>> >>> <https://pr-614--camel.netlify.app/components/latest/activemq-component.html#_path_parameters_2_parameters> >>> >>> <https://pr-614--camel.netlify.app/components/latest/activemq-component.html#_path_parameters_2_parameters >>> >>> <https://pr-614--camel.netlify.app/components/latest/activemq-component.html#_path_parameters_2_parameters>> >> >> I guess this is fixed now, looks okay to me... > > Yes, this is one of the many problems now fixed :-) >> >>> - All the names in the generated tables have ids so they can be linked to >>> but there’s no easy way to find out the id. The sections have a link to >>> themselves which makes it easy to copy the URL. I haven’t looked into >>> whether this can be done with the table entries. >> >> I think this is a nice to have, so we can definitely merge this >> without this and followup after... >> >>> Updating the main PR to track changes in camel is somewhat time consuming >>> so I’d appreciate it if we could move this towards resolution quickly. >> >> Yes, I'm looking into this right now, my goal is to get this merged _very_ >> soon! > > Thanks for the review!! > > David Jencks >> >> zoran >> -- >> Zoran Regvart
