Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updating Upstream snippets to workaround Downstream-tooling incapacitates with multiple-snippet bars #32263

Conversation

MichalMaler
Copy link
Contributor

@MichalMaler MichalMaler commented Mar 30, 2023

The cause of this PR is to solve issues caused by Downstream obsolete tooling that doesn't support advanced features.

This solution uses ifdef and ifndef to display correct content triggered by the presence/absence of the upstream context attribute. In the downstream repo, this attribute will be called downstream.

The intention here is to keep Upstream functionality untouched and recreate the snippers so that they can be reused for downstream efforts in the shape of tabs-as-list.

Description:
I am adding one attribute, context, and if condition to recreate G. Smet's brilliant snippets for multiple-option bars so that they display in a usable way in the downstream repo, too, since this advanced asciidoc is not supported there thanks to publication tool limitations.

The attribute, context, the if-condition combo was needed because of the fact that attributes cant transcend asciidoc markup elements (out of the box). However, the solution I created here is more elegant than the pure attribute one since I added just two rows of code to our upstream-attributes-docs file.

This results in completely the same Upstream output as G. Smet proposed, but now these files render properly in Downstream as a "good-looking" bullet-item list with some additional top-notch level styling.

@MichalMaler MichalMaler requested a review from gsmet March 30, 2023 10:04
@quarkus-bot quarkus-bot bot added this to To do in Quarkus Documentation Mar 30, 2023
@MichalMaler MichalMaler marked this pull request as draft March 30, 2023 10:08
@MichalMaler
Copy link
Contributor Author

Spot some replacements googledocs are doing to the original markup (oh bother, fixing). One sec

@MichalMaler MichalMaler force-pushed the QDOCS-209-Upstream-code-snippet-tweaks-for-downstream-synergy branch from c94aeb0 to 009989e Compare March 30, 2023 10:18
@MichalMaler
Copy link
Contributor Author

MichalMaler commented Mar 30, 2023

@gsmet Hello Guillaume. Can I ask you for a check? Builds correctly (local builds)

@github-actions
Copy link

github-actions bot commented Mar 30, 2023

🙈 The PR is closed and the preview is expired.

@MichalMaler MichalMaler force-pushed the QDOCS-209-Upstream-code-snippet-tweaks-for-downstream-synergy branch from 009989e to 3957951 Compare March 31, 2023 12:15
@MichalMaler MichalMaler marked this pull request as ready for review March 31, 2023 12:15
@MichalMaler MichalMaler changed the title Revamping Upstream snippets for synergy with downstream Updating Upstream snippets to workaround Downstream tooling incapacitates with multiple-snippet bars Mar 31, 2023
@MichalMaler MichalMaler changed the title Updating Upstream snippets to workaround Downstream tooling incapacitates with multiple-snippet bars Updating Upstream snippets to workaround Downstream-tooling incapacitates with multiple-snippet bars Mar 31, 2023
@MichalMaler MichalMaler force-pushed the QDOCS-209-Upstream-code-snippet-tweaks-for-downstream-synergy branch from 3957951 to e1ad3bd Compare April 3, 2023 10:04
@ebullient ebullient force-pushed the QDOCS-209-Upstream-code-snippet-tweaks-for-downstream-synergy branch from d5f9337 to 9aaa5bb Compare April 13, 2023 20:07
@@ -55,4 +55,8 @@
// .
:create-app-group-id: org.acme
// .
// Devtools snippet's attribute and context
:upstream:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would rather not use upstream/downstream, as we discussed. This PR is specifically about whether or not there is support for asciidoc tabs.
:asciidoc-tabs: true should be the default. :asciidoc-tabs: false can be set where tabs aren't supported, which can toggle flat behavior (which would apply to pdf also)

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ebullient You make a good point.

I think the original intention behind the upstream and downstream attributes was to use them to conditionalize a set of product-only changes located throughout the docs source. That way we could have a single attribute trigger a bunch of conditionals on stuff that we only need in product docs.

For the rest of product-only changes, we could achieve a similar result to the latter by using ifeval statements and an attribute named, for example, :target: that resolves to either community or rhbq.

IMHO, we should consider changing the attribute for this specific use case according to your suggestion.

If we want to avoid mentioning terms like upstream and downstream in the community source, or if we decide that the product-only stuff cannot live in the upstream source at all, we'll need to consider another substitution mechanism, such as parameterized attributes with 2 discreet sets of attribute values, one for the community docs and the other for RHBQ.

Notably, we also want to reduce the source-level difference between upstream and downstream docs altogether. Some of the product-specific stuff, like support statements can be moved to a more visible location downstream (for example to the RN, Component details or other resources).
Even so, we'll need parameterized attributes to substitute data like builder image names and the BOM GAV, as these differ between upstream and downstream, and must be listed accurately with respect to which version of the docs they are displayed in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@inoxx03 @ebullient This is not only about changing the attribute. If I apply this approach with a single attribute with two values based on repository context, it will no longer be an ifdev condition. I would need to rewrite all the snippets to use ifeval conditions instead. @maxandersen @gsmet , will this change help you in this process, or the current state is more about not touching original files and finding the other way?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think everyones life would be easier if we could just do it one way and then either transform it on HTML pages with javascript or some automatic conversion when moving it to some other publishing form.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think everyones life would be easier if we could just do it one way and then either transform it on HTML pages with javascript

I could agree with that but the problem is the destructive version is the one we need. We definitely lose some structure in the downstream case and I'm not sure we will be able to easily cover all the cases (and I personally don't have the time to check all the various cases we have).

If we don't go this way, I agree with @ebullient that :asciidoc-tabs: false would be better. It should be a simple search/replace to get this PR updated. I can do it if you don't feel like doing it.

Quarkus Documentation automation moved this from To do to Review pending Apr 13, 2023
@maxandersen
Copy link
Contributor

the current tab mechanics are simply doing blocks if there is no javascript in the HTML.
What is the actual difference in content is needed?

Feels like we should just be able to adapt that to be picked up by some changes to javascript?

@MichalMaler
Copy link
Contributor Author

the current tab mechanics are simply doing blocks if there is no javascript in the HTML. What is the actual difference in content is needed?

Feels like we should just be able to adapt that to be picked up by some changes to javascript?

@maxandersen
This was my proposal to tweak the upstream snippets so that they render as bullet-list blocks with proper rendering and styling docs team agreed. I am not a coder, so all I could do is to play with the asciidoc in these well-written snippets.

The best would be to ameliorate the downstream publishing system so that these changes would not be needed at all (but this will not change).
The motivation here was that writers would not have to edit the snippets with every downstreaming cycle again.
I tried to bring this to @gsmet attention, but his priority was/is the 3.0 release.

I just want to show what needs to be done so that these snippets render well in a publishing system lacking multiple-option-bar functionality. Again, I just propose a starting point for a conversation. Will be happy to test other upstream ideas that could solve this.

@maxandersen
Copy link
Contributor

Got an example somewhere on what bullet item style was agreed upon?

Is there a reason it must be a bullet list and that the current rendering does not suffice ?

Which outputs will have this form? Only pdf or?

@MichalMaler
Copy link
Contributor Author

MichalMaler commented May 15, 2023

Got an example somewhere on what bullet item style was agreed upon?

Is there a reason it must be a bullet list and that the current rendering does not suffice ?

Which outputs will have this form? Only pdf or?

Let me collect all the evidence; will ping you when ready. Thank you for your interest @maxandersen
Maybe a 5-minute-long meeting would be then better than replying to this thread. Not sure about the output, all I was caring about was how it looks in Pantheon.

So far, here is the screenshot and link to the PV2 preview build.

image

So, since we don't have a multiple-tab feature, we display it as a list.
This list is made so that the code and additional information are moved slightly to the right. Code and additional information are put into a block aligned as a subsection of each leading sentence with +.
Additional information is markup as a bullet list option, too, starting with ->* .

In code, I needed to split the asciidoc headers defining the heading and code spinnet separately so that it works as originally planned, even after my edits. (visible in the PR).

Each code snippet has a leading sentence.

Instead of CLI (since all options are CLI in the end), we are using By using Quarkus CLI (with URL to the upstream docs).

Since Quarkus CLI is not supported for production, we need to inform readers about it by adding a note that is not presented in the upstream version.

So, this is not only about different markups used but also about displaying additional information based on context (upstream/downstream).

@maxandersen
Copy link
Contributor

Are you saying that every place we show a quarkus cli command we'll have to show readers:

"The Quarks CLI is intended for development purposes only. Red Hat does not support using Quarkus CLI in production environments"

?

How is that useful when the exact same applies for mvn and gradle commands ?

And why not just say that once if absolutely necessary?

on the formatting - let me see if can find way to avoid needing different outputs.
Would an automatic transformation of the asciidoc be okey?

@MichalMaler
Copy link
Contributor Author

MichalMaler commented May 16, 2023

Hello Max! Good morning.
You may be right. I just looked at how this Quarkus CLI is handled in the other docs and guides and went with what I saw (I consulted this with QE, Pablo Gonzalez).

If the same applies to the mvn and Gradle, then the best solution would be to write a short paragraph about it and put it in the Release Notes, to the Support and Compatibility section, then delete it from the snippets, sure.

My intention was not to spam but not to bring anybody into trouble :)

https://pantheon.corp.redhat.com/pantheon/preview/latest/0f24d6b4-7032-4601-99cb-fbdefec89f6d?rerender=true#ref_rn-supported-features_quarkus-release-notes

Regarding the sub-level bullet points for additional information, we have some space to maneuver here too, and it can be deleted if necessary. I will ask the team about it and will try to reach Thomas and ask him about that CLI thing.

Regarding automatic transformation, every automation is welcomed. Do you plan to create a script to create other copies of these snippets for downstream use?

Update:

  1. Thomas agreed with my idea to delete it from the snippets and just mention it in the RN.
  2. I will remove bullet points from the additional commands mentioned below the sniped to achieve more clean look ;)

@inoxx03
Copy link

inoxx03 commented May 16, 2023

Hi, @maxandersen
@MichalMaler and I discussed possible ways of resolving this, too.
After Michal checked in with Thomas, we agreed to put the support-related information about Quarkus devtools in the RN and remove the notes from under the CLI command snippets. This will make downstreaming the markup a little easier.

To answer the questions from your previous comment:

Got an example somewhere on what bullet item style was agreed upon?

I don't think we've made a formal agreement on this, but I have traced usage of the bullet list format back to August 2022 when we first started implementing Diataxis templates upstream and experimenting with downstreaming. Here's the earliest instance I could find of the bulleted list format being used in the security guides: 9cece61

Erin and Sergey both ACK'd the change on the PR so we understood that as acceptance.

Is there a reason it must be a bullet list and that the current rendering does not suffice ?

TL;DR: The reason was to work around rendering limitations downstream (Please see Extended explanation for more details about why we use a bulleted list).

Does it have to be a bulleted list, specifically?
No, we did it because it was the least disruptive solution that required the least resources to implement and maintain and required no additional scripting/coding.

We can change it if we have a better or if the underlying limitation on the downstream side is no longer present.

Extended explanation:
Our downstream build system cannot render tabbed examples the way we show them upstream, so when we ported the markup directly and built the docs using downstream tools, the captions that are framed inside the AsciiDoc sidebar element that delineates the tabs in the upstream version of the guide were converted into paragraphs with no structure, the snippets themselves looked OK, but the "flattened" tabs rendered from unmodified markup were disorganized.
We realized that to render the markup downstream we need to make it transform into a list when flattened. @MichalMaler went ahead and tested several alternatives on how to do this. Adding the bullet points was the least disruptive one we could come up with at the time.

Which outputs will have this form? Only pdf or?

We are currently using the flattened form instead of tabs for HTML output downstream. (technically, the PDF output of downstream docs the would use it, too. Though we don't regularly ship PDFs of downstream docs at the moment, we can still build them on demand from source.)

Are you saying that every place we show a quarkus cli command we'll have to show readers:
"The Quarks CLI is intended for development purposes only. Red Hat does not support using Quarkus CLI in production environments"?

No, we only show that note once per document, usually the first time a Quarkus CLI command is shown. We don't repeat the note after subsequent instances of CLI examples.

How is that useful when the exact same applies for mvn and gradle commands ?

At the very basic level, it is there to protect from escalations.
However, I agree with you that there could be a better way of communicating this information.
You make a good point that the Maven and Gradle plugins are not supported (in the Red Hat product sense) either and should technically feature the same disclaimer as the Quarkus CLI examples. I believe this is something we can improve upon, and I come back to in my next reply.

And why not just say that once if absolutely necessary?

Our latest decision is to do exactly that - we are putting the information into the RHBQ release notes and removing it from under the snippets in guides.

On a related note, we also need a proper way to let RH customers know that Quarkus dev tools are not (and will not) be productized or supported on the same level that RHBQ Maven artifacts are.
To avoid escalations, we need to not only say that the dev tools are subject to Dev Support, but to also define what Dev support means in terms of commitment. IOW, we should be clear what kind of functionality we guarantee under the Dev Support scheme and what the intended usage of these tools is for Red Hat customers.
(To be clear, I understand that the tools work exceptionally well and are unlikely to be misused by anyone who know what they are doing. But escalations can also result from technicalities such as docs not being explicit enough about specific functionalities guaranteed by Red Hat. Example: someone uses the tool in a way that it is not intended to and to does not work the way they expect -> they file a ticket that we didn't provide this information clearly in the docs -> Red Hat must then assume responsibility and correct the problem.)

on the formatting - let me see if can find way to avoid needing different outputs. Would an automatic transformation of the asciidoc be okey?

Yes, automatic transformation would be suitable, too. We don't have the capacity in the docs team to write the transformation logic and maintain the job, so if you'd be willing to find such a solution, we'll be happy to test how it works:)

In the long term, my aim is to move our downstream docs off the current tooling and onto a system that is better maintained and much less limiting.

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Fixes

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Replacing hardcoded text to ifdefs

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Remake of the rest

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Adding conditionaled information-bullet attribute for Q-CLI

Signed-off-by: Michal Maléř <mmaler@redhat.com>

Update

Signed-off-by: Michal Maléř <mmaler@redhat.com>
@MichalMaler MichalMaler force-pushed the QDOCS-209-Upstream-code-snippet-tweaks-for-downstream-synergy branch from 9aaa5bb to 399d6cd Compare May 17, 2023 10:53
@MichalMaler
Copy link
Contributor Author

@maxandersen Also, about that And why not just say that once if absolutely necessary?
The screenshot I sent you was from manually edited downstream docs (and it was more about showcasing how it all looks). This was an ad-hoc solution to pass QE verification for 2.13.
As you can see in this PR, that sentence is applied only once and only in one snippet (create-cli.adoc, and only because I respect the original design of these files, this is not my intention). This means I plan to change this too since we all knew this was just a downstream hotfix, not a proper solution that should go through the upstream first.

@maxandersen
Copy link
Contributor

maxandersen commented May 22, 2023

idea discussed on quick cal today:
we could possibly do a tooled conversion of the _includes folder from the current form to the wanted output.
basically look for first line having "role="primary" if yes, do the conversion inplace.

Copy link
Contributor

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thus -1 on this current PR and even if I did manage to do some adjustments to the javascript converting it on the fly it is going to be very noisy for readers.

After discussing in more detail with @inoxx03 its clear that even if I could find a way to transform this it is only done to add a disclaimer which will be extremely noisy in the docs thus its inappropriate to have.

My suggestion is you at most per doc add something like:

ifdef::dev-support-disclaimer[]
{dev-support-disclaimer}
endif::[]

which you can then use to inject a disclaimer once rather than every time the dev tools are used.

Quarkus Documentation automation moved this from Review pending to Done May 30, 2023
@quarkus-bot quarkus-bot bot added the triage/invalid This doesn't seem right label May 30, 2023
@maxandersen
Copy link
Contributor

Closing this as the approach is not needed. @inoxx03 following up.

@MichalMaler
Copy link
Contributor Author

Closing this as the approach is not needed. @inoxx03 following up.

Thanks to all who participated in this effort.

@MichalMaler MichalMaler deleted the QDOCS-209-Upstream-code-snippet-tweaks-for-downstream-synergy branch May 31, 2023 09:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

5 participants