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

Highlight prerequisites line on amdgpu-install page #306

Merged
merged 6 commits into from
Oct 1, 2024

Conversation

alexxu-amd
Copy link
Contributor

Multiple feedbacks from internal and external mention the "set user group permission" command is not directly available from amdgpu-install page, which in fact, is included in the prerequisites page but tends to get ignored. Highlight the prerequisites line for improved visibility.

@alexxu-amd alexxu-amd requested a review from a team as a code owner September 30, 2024 18:13
@@ -22,7 +22,9 @@ Installation

Installation of ``amdgpu-install`` differs slightly depending on the OS and its package manager.

Make sure that the :doc:`/install/prerequisites` are met before installing.
.. note::
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
.. note::
.. important::

Do you want to try changing from "note" to "important"? Do we want it to standout more?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yea, I agree. Wasn't aware of the important block

@yhuiYH
Copy link
Collaborator

yhuiYH commented Sep 30, 2024

Also, to be complete, we should add a similar note on the "Ubuntu, SLES, and RHEL" install pages. Can you add that to your PR?

@alexxu-amd
Copy link
Contributor Author

Added the notes for Ubuntu, SLES, and RHEL. Also added some minor changes to unify the content:

  • RHEL refactors the installation part to install-rocm-template.rst, whereas the other two don't
  • RHEL uses regular tense on titles, the other two uses present tense
  • SLSE refers repo as "ROCm repositories", the other two just uses "repositories"

@@ -86,6 +86,10 @@ Add the ROCm repository.
Installing
================================================

.. important::
Copy link
Collaborator

Choose a reason for hiding this comment

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

@dbieleck Do you like where @alexxu-amd added it in the "Installing" part? Or do you think it should be right at the top of the page?

I think I'd like it right up at line 8, as the first thing they see, even before they start "registering the repositories".

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree. It might be better up at line 8 so it's visible right away.

Copy link
Collaborator

@yhuiYH yhuiYH left a comment

Choose a reason for hiding this comment

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

OK, this LGTM now. Approved.

@alexxu-amd alexxu-amd merged commit 8ddd04c into develop Oct 1, 2024
3 checks passed
@alexxu-amd alexxu-amd deleted the amd/alexxu12/highlight_prereq branch October 1, 2024 15:17
yhuiYH pushed a commit that referenced this pull request Oct 3, 2024
* highlight prerequisites block

* change note block to important block

* unifying native-install variants

* unify titles; add prereq important blocks for native install

* remove install-rocm-template.rst

* bring the note to the top
alexxu-amd added a commit that referenced this pull request Oct 3, 2024
* highlight prerequisites block

* change note block to important block

* unifying native-install variants

* unify titles; add prereq important blocks for native install

* remove install-rocm-template.rst

* bring the note to the top
yhuiYH pushed a commit that referenced this pull request Oct 3, 2024
* highlight prerequisites block

* change note block to important block

* unifying native-install variants

* unify titles; add prereq important blocks for native install

* remove install-rocm-template.rst

* bring the note to the top
alexxu-amd added a commit that referenced this pull request Oct 3, 2024
* Highlight prerequisites line on amdgpu-install page (#306)

* highlight prerequisites block

* change note block to important block

* unifying native-install variants

* unify titles; add prereq important blocks for native install

* remove install-rocm-template.rst

* bring the note to the top

* Update Virtualization support versions (#307)

* update Hypervisor versions

* fix RHEL version, and add EXSi details

* remove unnecessary template file (#312)

* Add tensorflow wheel versioning note (#303)

* build(deps): Bump rocm-docs-core from 1.7.1 to 1.8.2 in /docs/sphinx (#300)

Bumps [rocm-docs-core](https://github.com/ROCm/rocm-docs-core) from 1.7.1 to 1.8.2.
- [Release notes](https://github.com/ROCm/rocm-docs-core/releases)
- [Changelog](https://github.com/ROCm/rocm-docs-core/blob/v1.8.2/CHANGELOG.md)
- [Commits](ROCm/rocm-docs-core@v1.7.1...v1.8.2)

---
updated-dependencies:
- dependency-name: rocm-docs-core
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: Young Hui - AMD <[email protected]>
Co-authored-by: harkgill-amd <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
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.

3 participants