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

CP-45226: Added friendly name for new datasource 'hostload' #3243

Merged
merged 2 commits into from
Oct 25, 2023

Conversation

kc284
Copy link
Contributor

@kc284 kc284 commented Oct 24, 2023

No description provided.

@CitrixChris CitrixChris added the 1 approval PR has been approved by one reviewer label Oct 24, 2023
@@ -1856,6 +1856,9 @@
<data name="Message.name-wlb_vm_relocation" xml:space="preserve">
<value>WLB VM Relocation</value>
</data>
<data name="OPERATIONAL_METRICS_HOSTLOAD" xml:space="preserve">
<value>CPU Load</value>

Choose a reason for hiding this comment

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

Looks this will show on UI. So I suggest to use "Host Load" or "Host CPU Load" that keeps same style to Control Domain Load for loadavg

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I was wondering about that, then I thought I'd drop the "Host" bit because we don't use it in the other host CPU datasources, but I'll add it back. Thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Since it's similar to loadavg I also changed the category type shown by the UI.

XenModel/Utils/Helpers.cs Outdated Show resolved Hide resolved
@danilo-delbusso danilo-delbusso added the needs updating A reviewer has requested changes label Oct 25, 2023
@kc284 kc284 added updated Changes completed, please review ASAP PR should be reviewed as soon as possible and removed needs updating A reviewer has requested changes labels Oct 25, 2023
Copy link
Member

@danilo-delbusso danilo-delbusso left a comment

Choose a reason for hiding this comment

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

@danilo-delbusso danilo-delbusso added 2 approvals PR has been approved by two reviewers under review Do not merge, extra review requested by reviewer and removed 1 approval PR has been approved by one reviewer labels Oct 25, 2023
@kc284 kc284 merged commit fb9383d into xenserver:feature/nrpe Oct 25, 2023
1 check passed
@kc284 kc284 deleted the feature/nrpe branch October 25, 2023 10:43
@kc284 kc284 removed under review Do not merge, extra review requested by reviewer updated Changes completed, please review ASAP PR should be reviewed as soon as possible labels Oct 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 approvals PR has been approved by two reviewers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants