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

fix image location. #46

Merged
merged 2 commits into from
Mar 5, 2025
Merged

fix image location. #46

merged 2 commits into from
Mar 5, 2025

Conversation

jyao1
Copy link
Contributor

@jyao1 jyao1 commented Feb 25, 2025

The image location is invalid in original document.

Signed-off-by: Jiewen Yao <[email protected]>
@wmat
Copy link
Contributor

wmat commented Feb 25, 2025

The logo image comes from the docs-resources submodule and should be referenced from the document header with:

:imagesdir: ../docs-resources/images
:title-logo-image: image:risc-v_logo.png["RISC-V International Logo",pdfwidth=3.25in,align=center]

Note that if you'r building the PDF locally, you have to init the sub-modules directory to pull in the code.

@jyao1
Copy link
Contributor Author

jyao1 commented Feb 26, 2025

The logo image comes from the docs-resources submodule and should be referenced from the document header with:

:imagesdir: ../docs-resources/images :title-logo-image: image:risc-v_logo.png["RISC-V International Logo",pdfwidth=3.25in,align=center]

Note that if you'r building the PDF locally, you have to init the sub-modules directory to pull in the code.

OK. Updated patch according.

@tyshyu
Copy link
Contributor

tyshyu commented Feb 26, 2025

The logo image comes from the docs-resources submodule and should be referenced from the document header with:

:imagesdir: ../docs-resources/images :title-logo-image: image:risc-v_logo.png["RISC-V International Logo",pdfwidth=3.25in,align=center]

Note that if you'r building the PDF locally, you have to init the sub-modules directory to pull in the code.

The following is my suggestion for the document header:

:imagesdir: .
:title-logo-image: image:./docs-resources/images/risc-v_logo.svg["RISC-V International Logo",pdfwidth=3.25in,align=center]

It prevents :imagesdir: of the header file affects all .adoc file during building the PDF since some images related to the spec are in path ./images.

@jyao1
Copy link
Contributor Author

jyao1 commented Mar 5, 2025

The following is my suggestion for the document header:

:imagesdir: .
:title-logo-image: image:./docs-resources/images/risc-v_logo.svg["RISC-V International Logo",pdfwidth=3.25in,align=center]

It prevents :imagesdir: of the header file affects all .adoc file during building the PDF since some images related to the spec are in path ./images.

Yes, I have updated the patch according to this feedback.

iopmp.adoc Outdated
:imagesdir: images
:title-logo-image: image:risc-v_logo.svg[pdfwidth=3.25in,align=center]
:imagesdir: .
:title-logo-image: ./docs-resources/images/image:risc-v_logo.svg[pdfwidth=3.25in,align=center]
Copy link
Contributor

Choose a reason for hiding this comment

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

This line should be

:title-logo-image: image:./docs-resources/images/risc-v_logo.svg[pdfwidth=3.25in,align=center]

This line currently still outputs invalid image location during building PDF. (https://github.com/riscv-non-isa/iopmp-spec/actions/runs/13667321046/job/38211030383#step:4:12)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Updated. I hope it is correct this time.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great. I suppose the PR can be merged. @paul-andes

Signed-off-by: Jiewen Yao <[email protected]>
@paul-andes paul-andes merged commit 0be8a8a into riscv-non-isa:main Mar 5, 2025
1 check passed
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.

4 participants