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

rocm: add reason for disabled component #219

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

Conversation

dbarry9
Copy link
Contributor

@dbarry9 dbarry9 commented Aug 21, 2024

Pull Request Description

Previously, in the absence of a ROCm device, the rocm component did not set the string containing the reason that the component was disabled. This corresponds to Issue #220.

These changes have been tested using ROCm 6.2

Author Checklist

  • Description
    Why this PR exists. Reference all relevant information, including background, issues, test failures, etc
  • Commits
    Commits are self contained and only do one thing
    Commits have a header of the form: module: short description
    Commits have a body (whenever relevant) containing a detailed description of the addressed problem and its solution
  • Tests
    The PR needs to pass all the tests

Previously, in the absence of a ROCm device, the rocm component did not
set the string containing the reason that the component was disabled.

These changes have been tested using ROCm 6.2
@dbarry9 dbarry9 added the draft Do not merge (yet) label Aug 21, 2024
Copy link
Contributor

@Treece-Burgess Treece-Burgess left a comment

Choose a reason for hiding this comment

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

I personally would add a format specifier to snprintf, but I don't believe it is actually necessary in this case or beneficial since we are providing the error message.

The rest of the file seems to follow suit with no format specifier when we directly provide the error message as well.

@Treece-Burgess Treece-Burgess linked an issue Sep 28, 2024 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
draft Do not merge (yet)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

rocm: component disabled reason missing
2 participants