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

Fixes #37689 - Add NVME controllers to hammer help #628

Merged
merged 1 commit into from
Jul 26, 2024

Conversation

girijaasoni
Copy link
Contributor

@girijaasoni girijaasoni commented Jun 3, 2024

This is to update the hammer documentation to accodomate NVME controllers.
It can be merged after foreman PR and vpshere pr

RM Link https://projects.theforeman.org/issues/37689

Copy link

@Lennonka Lennonka left a comment

Choose a reason for hiding this comment

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

Generally LGTM, just one question below.

['boot_order', _('Device names to specify the boot order')]
' key - ' + _('Key of the controller (e.g. 1000)')].flatten(1).join("\n")],
['nvme_controllers', [_('List with NVME controllers definitions'),
' type - ' + _('ID of the controller from VMware'),
Copy link

@Lennonka Lennonka Jun 22, 2024

Choose a reason for hiding this comment

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

It looks a bit strange that it's called a "type", but it requires an identifier. Is it an identifier of a controller type or a particular controller?

Suggested change
' type - ' + _('ID of the controller from VMware'),
' type - ' + _('ID of the controller type from VMware'),

@ofedoren
Copy link
Member

Thanks, @girijaasoni, could you please create a RM ticket in hammer project to attach it to this PR and https://projects.theforeman.org/issues/34839?

@girijaasoni girijaasoni changed the title Add NVME controllers to hammer help Fixes #37689 - Add NVME controllers to hammer help Jul 26, 2024
@girijaasoni
Copy link
Contributor Author

Thanks, @girijaasoni, could you please create a RM ticket in hammer project to attach it to this PR and https://projects.theforeman.org/issues/34839?

done

@ofedoren ofedoren merged commit 02a38bb into theforeman:master Jul 26, 2024
6 checks 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.

3 participants