-
Notifications
You must be signed in to change notification settings - Fork 238
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
Create a Validate pkg #1455
Create a Validate pkg #1455
Conversation
✅ Deploy Preview for kubernetes-sigs-nfd ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
d511a1f
to
82bbc05
Compare
oh I forgot unit tests at master 😓 |
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.
Thanks @ArangoGutierrez for the PR. Splitting this out of #1446 makes for a lot easier review. I'd keep this patch validation-only and have the dynamic value expansion of labels inside nfd-master
22dc311
to
3bd2ecd
Compare
3bd2ecd
to
a341513
Compare
@AhmedGrati @marquiz PTAL |
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.
Nice improvement @ArangoGutierrez!
/assign @marquiz |
a341513
to
456217d
Compare
@marquiz @AhmedGrati Rebased, ready for another round |
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.
@ArangoGutierrez I think it would be a great addition if you can add some unit tests for the validate
functions. wdyt?
456217d
to
048814b
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! Thanks for your efforts @ArangoGutierrez!
11071d3
to
5243e91
Compare
5243e91
to
f35bb58
Compare
f35bb58
to
acab880
Compare
acab880
to
b397eb3
Compare
Oh, and this, i.e. remove the mention that the default namespaces cannot be denied |
b397eb3
to
e585849
Compare
Signed-off-by: Carlos Eduardo Arango Gutierrez <eduardoa@nvidia.com>
e585849
to
affb93e
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.
Thanks @ArangoGutierrez for the contribution. Looks ready for me
/lgtm
LGTM label has been added. Git tree hash: 125fa1e7f300f2a0b1e17b1402b6de5ac44d5c1b
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: AhmedGrati, ArangoGutierrez, marquiz The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
/retest |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #1455 +/- ##
==========================================
+ Coverage 30.75% 31.49% +0.73%
==========================================
Files 58 59 +1
Lines 7550 7621 +71
==========================================
+ Hits 2322 2400 +78
+ Misses 4977 4972 -5
+ Partials 251 249 -2
|
No description provided.