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

issue #2169 - change getValueConstant to getValueAsEnum and update project code #2293

Merged
merged 4 commits into from
Apr 28, 2021

Conversation

lmsurpre
Copy link
Member

@lmsurpre lmsurpre commented Apr 27, 2021

The first commit has the model change (getValueConstant -> getValueAsEnum) and not much else.
I suggest focusing on the second commit for the review.

Signed-off-by: Lee Surprenant lmsurpre@us.ibm.com

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre force-pushed the lee-main branch 2 times, most recently from 33085a9 to 95d0533 Compare April 27, 2021 03:22
Copy link
Contributor

@prb112 prb112 left a comment

Choose a reason for hiding this comment

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

LGTM - left two comments... one about a nice change, and the other about applying the change in a different spot potentially.

@lmsurpre lmsurpre force-pushed the lee-main branch 2 times, most recently from d699abe to caa436f Compare April 27, 2021 13:06
@lmsurpre lmsurpre requested a review from tbieste April 27, 2021 13:09
Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
And convert from for loop to a switch.

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
Copy link
Collaborator

@JohnTimm JohnTimm left a comment

Choose a reason for hiding this comment

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

LGTM

@lmsurpre lmsurpre merged commit a46fa00 into main Apr 28, 2021
@lmsurpre lmsurpre deleted the lee-main branch April 28, 2021 14:38
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.

None yet

3 participants