-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Making verbosity list and options a constant and adding WinRM debug #12378
Conversation
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.
This works as-is, however if you wanted to use just one constant variable instead of having to maintain two I would suggest creating a mapper function to return the value (label
) you want for the details views.
awx/ui/src/constants.js
Outdated
@@ -12,3 +14,18 @@ export const SESSION_TIMEOUT_KEY = 'awx-session-timeout'; | |||
export const SESSION_REDIRECT_URL = 'awx-redirect-url'; | |||
export const PERSISTENT_FILTER_KEY = 'awx-persistent-filter'; | |||
export const SESSION_USER_ID = 'awx-session-user-id'; | |||
|
|||
export const VERBOSITY = { |
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.
Good example of standardization here. If you wanted to just use one constant to represent Verbosity and it's permutations in the UI, I could see creating the object literal containing the key
, value
, and label
keys and then calling a function to return the appropriate label when not used as an options dropdown.
export const getVerbosity = (key) => VERBOSITY.filter(v => v.key === key)
const [{ label }] = getVerbosity(verbosityKey);
You could probably just save both the constant and the function as one object literal itself.
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.
Those strings are not being selected to be translated.
I followed up with @john-westcott-iv offline.
b32c5d2
to
ef8d4e7
Compare
Dismissing @nixocio's review to enable merging.
SUMMARY
ISSUE TYPE
COMPONENT NAME
AWX VERSION
ADDITIONAL INFORMATION