-
Notifications
You must be signed in to change notification settings - Fork 2.2k
feat!: Add support for enterprise rulesets #3417
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3417 +/- ##
==========================================
- Coverage 97.72% 92.26% -5.46%
==========================================
Files 153 174 +21
Lines 13390 15023 +1633
==========================================
+ Hits 13085 13861 +776
- Misses 215 1068 +853
- Partials 90 94 +4 ☔ View full report in Codecov by Sentry. |
gmlewis
left a comment
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.
Thank you, @stevehipwell .
In addition to the linter findings, please make the following changes:
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
2cfa3e7 to
6ce3f9b
Compare
|
Please don't use force-push to PRs in this repo as explained in CONTRIBUTING.md, as it makes it difficult for reviewers to see what has changed since the last review. We always use squash-and-merge in this repo, so commit history will always be clean. Thank you for your consideration and understanding. |
|
@gmlewis sorry for the force pushing, it's muscle memory. |
|
@gmlewis based on the last run it looks like the OpenAPI schema needs to be updated? |
I'll see if I can update them locally and report back my findings. |
@stevehipwell - I just created #3419. Could you please see if this looks right to you? |
|
@gmlewis how would you like me to update this PR so the metadata is correct? Usually I'd rebase the commit onto the repo changes. |
If you |
Done, but will GH give you a linear history on squash? |
Squash gives us a very clean linear history of PRs. Check out this repo's |
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
gmlewis
left a comment
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.
One more nit... would you mind renaming the parameter rs Ruleset to ruleset Ruleset in this PR to make the docs a bit more readable, please?
Oh, and with that last comment I sent - if you still want to change the signature to the deprecated endpoint for consistency, that is fine with me... so maybe just update the comment and ignore the rest.
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
Signed-off-by: Steve Hipwell <steve.hipwell@gmail.com>
gmlewis
left a comment
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.
Thank you, @stevehipwell !
LGTM.
Awaiting second LGTM+Approval from any other contributor to this repo before merging.
dnwe
left a comment
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.
feature implementation looks good to me and matches the docs
|
Thank you, @dnwe ! |
BREAKING CHANGE:
Create*RulesetandUpdate*Rulesetnow passrulesetparameter by-value instead of by-reference.Fixes: #3416.