-
Notifications
You must be signed in to change notification settings - Fork 5.3k
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
add more information when service.platform
isn't part of service.build.platforms
#9868
Conversation
@bkielbasa can you sign-off your commit? Just follow the instructions here |
@bkielbasa you need to rebase your branch and update the e2e tests as I mentioned before, otherwise we're almost ready to merge it! |
@glours thanks for your review! I updated tests so it should be ready to go! |
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.
As your branch isn't up to date with the v2 branch, we can't merge your PR or we'll introduce potential regression.
The only changes we should see in this PR are:
- The 2 updates of the logs
- The 2 updates in the e2e test assertions
…d.platforms Signed-off-by: Bartłomiej Klimczak <bartlomiej.klimczak88@gmail.com>
Signed-off-by: Bartłomiej Klimczak <bartlomiej.klimczak88@gmail.com>
@glours done! Sorry, I mess up with rebase :) it should be fine now |
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, thanks for your contribution
What I did
I found it a bit problematic to debug so I added a bit more detailed error messages when
service.platform
isn't part ofservice.build.platforms
.