-
Notifications
You must be signed in to change notification settings - Fork 42
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 improvements on subscription with activation key #2396
add improvements on subscription with activation key #2396
Conversation
Signed-off-by: Adelia Ferreira <[email protected]>
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #2396 +/- ##
==========================================
+ Coverage 62.58% 62.74% +0.15%
==========================================
Files 88 88
Lines 13380 13409 +29
==========================================
+ Hits 8374 8413 +39
+ Misses 4323 4313 -10
Partials 683 683 ☔ View full report in Codecov by Sentry. |
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 love that you created a helper function! Left a question for you. I was a bit confused with the tests, but I don't have time to review the entire test suite (maybe some of the context I'm missing is already set somewhere else?).
This pull request needs all conversation threads to be resolved. Could you fix it @adeliaferreira? 🙏 |
Signed-off-by: Adelia Ferreira <[email protected]>
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.
looks good , left some comments.
Co-authored-by: Djebran Lezzoum <[email protected]>
Co-authored-by: Djebran Lezzoum <[email protected]>
Co-authored-by: Djebran Lezzoum <[email protected]>
Signed-off-by: Adelia Ferreira <[email protected]>
This pull request needs all conversation threads to be resolved. Could you fix it @adeliaferreira? 🙏 |
Signed-off-by: Adelia Ferreira <[email protected]>
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, many thanks Tay
Description
After hot fixes from last week, this PR will address some of comments we have open on subscription to image.
It's create a new func that will create the subscription object to fill "req.customizations.subscription" if adequate and more tests to validate it.
FIXES:
Type of change
What is it?