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

Conditionalize content in upstream Quarkus repository for the 3.8.next product release #39999

Merged
merged 1 commit into from
May 22, 2024

Conversation

rolfedh
Copy link
Contributor

@rolfedh rolfedh commented Apr 10, 2024

Fixes #39954

Copy link

quarkus-bot bot commented Apr 10, 2024

Thanks for your pull request!

The title of your pull request does not follow our editorial rules. Could you have a look?

  • title should not end up with ellipsis (make sure the title is complete)

This message is automatically generated by a bot.

@quarkus-bot quarkus-bot bot added area/docstyle issues related for manual docstyle review area/documentation labels Apr 10, 2024

This comment has been minimized.

@rolfedh rolfedh force-pushed the conditionalize-security-content branch from 1e77253 to ff5b29b Compare April 10, 2024 22:30

This comment has been minimized.

@rolfedh rolfedh force-pushed the conditionalize-security-content branch from ff5b29b to 77d7107 Compare April 10, 2024 22:58

This comment has been minimized.

@rolfedh rolfedh force-pushed the conditionalize-security-content branch from 77d7107 to c953567 Compare April 11, 2024 03:09

This comment has been minimized.

@sberyozkin sberyozkin changed the title Conditionalize content in upstream Quarkus repository for the 3.8.nex… Conditionalize content in upstream Quarkus repository for the 3.8.next product release Apr 11, 2024
@sberyozkin
Copy link
Member

Hi @rolfedh It is a massive and tricky project, thanks, I've started making suggestions, but then stopped as I thought #ifdef extension allows a given extension and #ifndef disallows it, or is it the other way around in the Doc context ?

Copy link
Contributor Author

@rolfedh rolfedh left a comment

Choose a reason for hiding this comment

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

Thank you for your review suggestions, @sberyozkin! Very appreciated!
For each suggestion, I've either updated the content and marked it as resolved, or I've provided a response in a comment. Please would you have at these unresolved ones and if you agree with my comment, mark them as resolved?

This comment has been minimized.

This comment has been minimized.

@rolfedh
Copy link
Contributor Author

rolfedh commented Apr 11, 2024

Hi @rolfedh It is a massive and tricky project, thanks, I've started making suggestions, but then stopped as I thought #ifdef extension allows a given extension and #ifndef disallows it, or is it the other way around in the Doc context ?

Understandable confusion. It's slightly counterintuitive.
The ifndef blocks display content in the community docs and hide it in the product docs.
I've provided the reason for this approach here: #39999 (comment)

Copy link

quarkus-bot bot commented Apr 11, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 6a423e0.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

⚠️ There are other workflow runs running, you probably need to wait for their status before merging.

You can consult the Develocity build scans.

This comment has been minimized.

@rolfedh rolfedh force-pushed the conditionalize-security-content branch from 6d8e5c8 to 91bdad2 Compare May 14, 2024 19:41

This comment has been minimized.

@rolfedh rolfedh requested review from sberyozkin and rsvoboda May 15, 2024 16:25
@rolfedh rolfedh force-pushed the conditionalize-security-content branch from 57a8553 to e12f035 Compare May 15, 2024 16:49

This comment has been minimized.

Copy link
Member

@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.

I was about to +1 until i noticed this touches more than just security docs and remove important info around in writing extensions. see my comments.

@rolfedh rolfedh force-pushed the conditionalize-security-content branch 2 times, most recently from de5c046 to 1e19981 Compare May 16, 2024 16:46
@rolfedh rolfedh requested a review from maxandersen May 16, 2024 17:13

This comment has been minimized.

@rolfedh rolfedh force-pushed the conditionalize-security-content branch from 1e19981 to 3285287 Compare May 16, 2024 20:04
Copy link

quarkus-bot bot commented May 16, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 3285287.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

@maxandersen maxandersen merged commit 2d99766 into quarkusio:main May 22, 2024
5 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.12 - main milestone May 22, 2024
@quarkus-bot quarkus-bot bot added the kind/enhancement New feature or request label May 22, 2024
@sberyozkin
Copy link
Member

Thanks @rolfedh

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/docstyle issues related for manual docstyle review area/documentation area/oidc area/vertx kind/enhancement New feature or request triage/qe? Issue could use a quality focused review to further harden it
Projects
Development

Successfully merging this pull request may close these issues.

Conditionalize content in upstream Quarkus repository for the 3.8.next product release
5 participants