-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
list VMs by displayname instead of name #8503
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## 4.18 #8503 +/- ##
=============================================
+ Coverage 13.12% 30.73% +17.60%
- Complexity 9142 33911 +24769
=============================================
Files 2720 5341 +2621
Lines 257744 374971 +117227
Branches 40182 54546 +14364
=============================================
+ Hits 33839 115242 +81403
- Misses 219615 244488 +24873
- Partials 4290 15241 +10951
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@DaanHoogland |
Or not, to be discussed |
36be215
to
8cb9dcf
Compare
1 similar comment
@sureshanaparti a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
@shwstppr a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
clgtm
8cb9dcf
to
36be215
Compare
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
As far as I know that weren't regressions; As part of #7833 since the display and name are the same for most users, having two redundant columns - so one of the two were removed as part of polish idea on #7833: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually prefer name to display name; but I've no objection if everybody agrees with the changes. Historical context: for many resources we've a name and a displayname, which is because CPBM and other portal/layers could refer to a resource by its display name while internally in ACS it could be referred to as name.
@andrijapanicsb can you give your uncensored input on this? |
ping @andrijapanicsb |
I did RTFM the whole thing, and the original issue/comments. LGTM - I think redundancy makes no sense (having both fields), especially since VMs created by UI have identical values for both. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## 4.18 #8503 +/- ##
============================================
- Coverage 12.24% 12.24% -0.01%
+ Complexity 9291 9290 -1
============================================
Files 4698 4698
Lines 414259 414259
Branches 51377 51765 +388
============================================
- Hits 50707 50706 -1
- Misses 357251 357252 +1
Partials 6301 6301
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
36be215
to
7faa31c
Compare
@DaanHoogland a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
@kiranchavala a Jenkins job has been kicked to build UI QA env. I'll keep you posted as I make progress. |
UI build: ✔️ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* 4.18: list by displayname instead of name (#8503)
@DaanHoogland is name used for instances when displayname may be empty or not returned by the API? (or perhaps is it handled by the backend API?) |
display name is filled with name is not provides as far as I know, I'll check the code. |
Description
This PR addresses #4654 (comment) TBD
Types of changes
Feature/Enhancement Scale or Bug Severity
Feature/Enhancement Scale
Bug Severity
Screenshots (if appropriate):
How Has This Been Tested?
How did you try to break this feature and the system with this change?
in qa