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

FCCEUD-80 Content checklist for Pantheon 2/Jupiter #627

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RichardHoch
Copy link
Contributor

Resolves https://issues.redhat.com/browse/FCCEUD-80

Adds content checklist for Pantheon 2/Jupiter to online help.

@codecov-commenter
Copy link

codecov-commenter commented Nov 10, 2021

Codecov Report

Merging #627 (3e76786) into master (94360c0) will decrease coverage by 0.41%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #627      +/-   ##
============================================
- Coverage     52.85%   52.44%   -0.42%     
+ Complexity      528      527       -1     
============================================
  Files           156      154       -2     
  Lines          5165     5076      -89     
  Branches        799      785      -14     
============================================
- Hits           2730     2662      -68     
+ Misses         2300     2287      -13     
+ Partials        135      127       -8     
Flag Coverage Δ
java 52.44% <ø> (-0.42%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...ntheon/validation/helper/XrefValidationHelper.java 50.00% <0.00%> (-27.78%) ⬇️
...n-bundle/frontend/src/app/BulkOperationPublish.tsx 36.08% <0.00%> (-0.09%) ⬇️
pantheon-bundle/frontend/src/app/Utils.tsx 22.72% <0.00%> (ø)
pantheon-bundle/frontend/src/app/search.tsx 36.15% <0.00%> (ø)
...dle/frontend/src/app/contexts/GitImportContext.tsx 7.40% <0.00%> (ø)
...m/redhat/pantheon/servlet/PublishDraftVersion.java 46.25% <0.00%> (ø)
...edhat/pantheon/asciidoctor/AsciidoctorService.java 3.75% <0.00%> (ø)
...t/pantheon/validation/helper/ValidationHelper.java 0.00% <0.00%> (ø)
.../pantheon/validation/validators/XrefValidator.java 0.00% <0.00%> (ø)
...pantheon/servlet/DocumentVariantRenderServlet.java 96.55% <0.00%> (ø)
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 94360c0...3e76786. Read the comment docs.

@stoobie
Copy link
Contributor

stoobie commented Nov 10, 2021

Resolves https://issues.redhat.com/browse/FCCEUD-80

Adds content checklist for Pantheon 2/Jupiter to online help.

@RichardHoch

Please add FCCEUD-80 to the beginning of the PR title. It makes it easier for me to search for FCCEUD PRs.

@RichardHoch RichardHoch changed the title WIP: Content checklist for Pantheon 2/Jupiter WIP: FCCEUD-80 Content checklist for Pantheon 2/Jupiter Nov 10, 2021
@RichardHoch RichardHoch marked this pull request as ready for review November 17, 2021 14:28
@RichardHoch RichardHoch requested a review from a team January 20, 2022 08:54
Copy link

@noopurmath3 noopurmath3 left a comment

Choose a reason for hiding this comment

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

Looks ok to me. However, I would like @xdavidson to have a look as well

*Not supported*

`:leveloffset: +1
modules/core-services/proc_registering-the-system-after-the-installation.adoc[leveloffset: -1]`

Choose a reason for hiding this comment

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

Looks like include:: is missing at the beginning of this line

Copy link
Collaborator

Choose a reason for hiding this comment

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

@noopurmath3 , it does look like 'include::' is missing on line 34

Copy link
Contributor Author

@RichardHoch RichardHoch Jan 20, 2022

Choose a reason for hiding this comment

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

I copied what was written in https://docs.engineering.redhat.com/pages/viewpage.action?pageId=173289387 -- see the "Avoid Unsupported includes" section. I am checking with the person who wrote the document and will copy you. .

*Not supported*

`:leveloffset: +1
modules/core-services/proc_registering-the-system-after-the-installation.adoc[leveloffset: -1]`
Copy link

@Levi-Leah Levi-Leah Jan 20, 2022

Choose a reason for hiding this comment

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

Lisa is correct, the include is missing. Also, the leveloffset: -1 should be on it's own line

Suggested change
modules/core-services/proc_registering-the-system-after-the-installation.adoc[leveloffset: -1]`
modules/core-services/proc_registering-the-system-after-the-installation.adoc
leveloffset: -1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great --- will fix and resubmit.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Like this? Maybe I didn't understand -- it still looks odd to me.

Not supported

:leveloffset: +1 include::modules/core-services/proc_registering-the-system-after-the-installation.adoc leveloffset: -1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

:leveloffset: +1 + include::modules/core-services/proc_registering-the-system-after-the-installation.adoc + leveloffset: -1

GitHub is erasing my hard returns (+)

Copy link

@Levi-Leah Levi-Leah Jan 20, 2022

Choose a reason for hiding this comment

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

Like this? Maybe I didn't understand -- it still looks odd to me.

Not supported

:leveloffset: +1 include::modules/core-services/proc_registering-the-system-after-the-installation.adoc leveloffset: -1

In asciidoc, if you have a bunch of includes with no leveloffset like include::some-module.adoc[] and put them in between :leveloffset: +1 and :leveloffset: -1, they will all inherit leveloffset=+1, as if you had all of the includes formatted like this include::modules/core-services/proc_registering-the-system-after-the-installation.adoc[leveloffset=+1]. Unfortunately, it’s not supported as far as we know.

link to asciidoc manual https://docs.asciidoctor.org/asciidoc/latest/directives/include-with-leveloffset/

hope this clarifies

@RichardHoch RichardHoch changed the title WIP: FCCEUD-80 Content checklist for Pantheon 2/Jupiter FCCEUD-80 Content checklist for Pantheon 2/Jupiter Feb 9, 2022
@RichardHoch
Copy link
Contributor Author

@xdavidson and @Levi-Leah, please check lines 33 and forward.

@xdavidson
Copy link
Collaborator

@xdavidson and @Levi-Leah, please check lines 33 and forward.

xref support in pantheon 2 is different from that in Jupiter. Pantheon2 implemented its own xref macro to use path based xref target when modules are included in an assembly. This is an unsupported bahviour in upstream asciidoctor community.
Here is what Jupiter supports in regards to xref today: https://docs.engineering.redhat.com/display/CCS/Xref+Support

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants