-
Notifications
You must be signed in to change notification settings - Fork 25
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
feat(STONEINTG-1006): adding to component status.lastPromotedImage #86
feat(STONEINTG-1006): adding to component status.lastPromotedImage #86
Conversation
be75942
to
1e83ffc
Compare
@Katka92 pinging you here for review and awareness in light of the discussion yesterday about contract testing. |
1e83ffc
to
cc0e529
Compare
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.
LGTM
@Katka92 any concerns with this before we merge it? |
@ralphbean Thank you for pinging me. @rohitkrai03 can you please confirm it is not a breaking change for the UI? |
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.
LGTM other than the trailing space nit mentioned by Jiri
Signed-off-by: Kasem Alem <[email protected]>
cc0e529
to
f4ac470
Compare
@rohitkrai03 , @Katka92 we still waiting for a confirmation if it's safe to merge this PR , can you please update ? |
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.
/approve
@kasemAlem Its safe to merge this PR. I don't see it causing any issues on the UI side as its just adding a new field in status. |
Adding new status field lastPromotedImage , for more details review in STONEINTG-1006