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.

Sign up for GitHub

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

Jump to bottom

Fix bad numbering format (.1.) for section numbers (sectnums) and some... #36928

Merged
merged 1 commit into from
Nov 30, 2023
Merged

Fix bad numbering format (.1.) for section numbers (sectnums) and some... #36928

merged 1 commit into from
Nov 30, 2023

Conversation

michelle-purcell
Copy link
Contributor

@michelle-purcell michelle-purcell commented Nov 7, 2023 •
edited
Loading

This PR fixes the problem of an extra dot (.) before each section number, which appears in tutorials with :sectnums: enabled.

image

and

image

This update:

  • Adds the :numbered: AsciiDoc attribute to the doc header of a tutorial and the tutorial template file to enforce the Arabic number format of 1. 2. 3. etc etc instead of .1. .2. .3. etc, which sometimes occurs depending on the markup.
  • Fixes the two OIDC security tutorials, which have bad numbering rendered on the doc site.

image

  • Also fixes a code snippet rendering issue spotted by @gsmet during this review.

@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Nov 7, 2023
@michelle-purcell
Copy link
Contributor Author

@sheilamjones - Thanks so much for flagging this problem 👍
The fix in this PR looks good both up and downstream from what I have tested locally.

@michelle-purcell
Copy link
Contributor Author

@maxandersen @aloubyansky - When this update is merged to main, please backport to 3.2 for product docs. It's a doc bug fix for both upstream and downstream doc rendering. Thanks so much 🙏

@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 7, 2023 •
edited
Loading

🙈 The PR is closed and the preview is expired.

@gsmet
Copy link
Member

gsmet commented Nov 7, 2023

From what I can see, this is not the only problem with this guide on the website.

The tabs are broken in the Using Maven (pom.xml) and Using Gradle (build.gradle) sections.

My understanding was that you were supposed to use the common pattern we had everywhere else and have a transformation to make it compatible with downstream but I see that you are using a completely different pattern that is breaking the Javascript.

@gsmet
Copy link
Member

gsmet commented Nov 7, 2023

See
2023-11-07 22 07 00 7b5fd49843c1

@sheilamjones
Copy link
Contributor

sheilamjones commented Nov 7, 2023 •
edited
Loading

@gsmet, thanks for your review. This PR is to address the numbering issue only.
I am finalizing a separate PR to address final edits/Vale fixes in this Bearer token guide (https://github.com/quarkusio/quarkus/pull/36933/files). I noticed the issue with this code snippet also and for the purposes of having an atomic commit, shall address that code snippet issue there. I hope this okay.

@michelle-purcell
Copy link
Contributor Author

michelle-purcell commented Nov 8, 2023 •
edited
Loading

@gsmet - Thanks for reviewing and the above ^^ 👍
+1 to comment ^^ from @sheilamjones. Can we handle the other issue in a different PR? Thanks.

@michelle-purcell michelle-purcell force-pushed the doc-numbering-fix branch 5 times, most recently from 2962162 to 77760c7 Compare November 8, 2023 17:53
@quarkus-bot quarkus-bot bot added area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/qute The template engine labels Nov 8, 2023
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@michelle-purcell michelle-purcell force-pushed the doc-numbering-fix branch 2 times, most recently from a933c66 to c080e04 Compare November 9, 2023 10:47
@quarkus-bot quarkus-bot bot added the area/dependencies Pull requests that update a dependency file label Nov 9, 2023
@maxandersen
Copy link
Member

See

which guide is that from ? didn't spot such rendering on the preview or in quarkus.io?

maxandersen
maxandersen reviewed Nov 9, 2023
View reviewed changes
docs/src/main/asciidoc/_templates/template-tutorial.adoc Outdated
@@ -12,6 +12,7 @@ TODO:
[id="..."]
= Title using sentence capitalization
include::_attributes.adoc[]
:numbered:
Copy link
Member

Choose a reason for hiding this comment

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

is this issue of :numbered: not general?

should it be set in pom.xml attributes section instead?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, that's a valid concern, any thoughts @michelle-purcell ?

Copy link
Member

Choose a reason for hiding this comment

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

@michelle-purcell could you please respond to the above question?

Copy link
Contributor Author

@michelle-purcell michelle-purcell Nov 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@maxandersen - Sorry, I missed this Q...

Yes, I think you are right @maxandersen - Adding the header attribute numbered fixed the .1. numbering part of the issue but wasn't enough to fix the indentation level 3-4 part of the numbering issue. The latest edits also resolve the numbering format issue too. I just tested without that attribute and the numbering works fine locally so I removed it from the 2 modified topics and the tutorial template. Thanks ⭐

@gastaldi @aloubyansky @sheilamjones - FYI.

@michelle-purcell michelle-purcell force-pushed the doc-numbering-fix branch 2 times, most recently from 168ae7d to 36f16fd Compare November 9, 2023 15:18
@michelle-purcell
Copy link
Contributor Author

@gsmet / @maxandersen / @aloubyansky - In the interest of closing this and getting the topic to QE ASAP for 3.2.8 product doc release, I fixed the code snippet issue that @gsmet raised during this review in this PR. Can we merge and backport now?
Thanks all.

@michelle-purcell michelle-purcell changed the title Fix bad numbering format (.1.) for section numbers (sectnums) Fix bad numbering format (.1.) for section numbers (sectnums) and some... Nov 9, 2023
@quarkus-bot quarkus-bot

This comment has been minimized.

Sign in to view
@sheilamjones
Copy link
Contributor

Hi @gsmet / @maxandersen / @aloubyansky, would it be possible to merge and backport this PR please as a separate PR with some additional Vale fixes for this same topic is dependent on it?
Many thanks,
Sheila

@aloubyansky
Copy link
Member

I am planning another backport round tomorrow. But I'd like somebody more familiar with the doc formatting review this one. Thanks.

@michelle-purcell
Copy link
Contributor Author

Removing backport labels as we are no longer publishing this guide on 3.2 for product. Instead we will improve on main.

@michelle-purcell
Copy link
Contributor Author

michelle-purcell commented Nov 16, 2023 •
edited
Loading

@aloubyansky / @gastaldi / @gsmet gsmet - Please approve and merge the content fixes. We no longer need this to be backported but it would be good to make the fix in main as the doc defect exists here and needs to be fixed. Thanks.

@sheilamjones
Copy link
Contributor

Hi @aloubyansky / @gastaldi / @gsmet, would it be possible to please approve and merge these fixes in Main? Note that we no longer need to backport this, but it would be good to make these fixes in main.
Many thanks.

@michelle-purcell michelle-purcell force-pushed the doc-numbering-fix branch 2 times, most recently from 06b892d to 6e5f781 Compare November 22, 2023 09:39
@quarkus-bot quarkus-bot bot added area/hibernate-orm Hibernate ORM area/persistence OBSOLETE, DO NOT USE labels Nov 22, 2023
@michelle-purcell
Fix bad numbering format for sectnums
85481e5 
Fix

Fix other tutorial

remove html output

fix code block

Removing :numbered: from header
@michelle-purcell
Copy link
Contributor Author

@maxandersen - All comments are now resolved. Can we merge this one to main? Thanks.

@gastaldi gastaldi merged commit c233d06 into quarkusio:main Nov 30, 2023
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.7 - main milestone Nov 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gastaldi gastaldi gastaldi approved these changes

@maxandersen maxandersen maxandersen left review comments

@sheilamjones sheilamjones Awaiting requested review from sheilamjones

@aloubyansky aloubyansky Awaiting requested review from aloubyansky

Assignees
No one assigned
Labels
area/dependencies Pull requests that update a dependency file area/docstyle issues related for manual docstyle review area/documentation area/hibernate-orm Hibernate ORM area/infra-automation anything related to CI, bots, etc. that are used to automated our infrastructure area/persistence OBSOLETE, DO NOT USE area/qute The template engine area/tracing
Projects
Quarkus Documentation
Status: Done
Milestone
3.7.0.CR1
Development

Successfully merging this pull request may close these issues.
6 participants
@michelle-purcell @gsmet @sheilamjones @maxandersen @aloubyansky @gastaldi