Skip to content
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

refactor: Remove client.Reset #169

Merged
merged 2 commits into from
Dec 18, 2021

Conversation

willbeason
Copy link
Member

@willbeason willbeason commented Dec 16, 2021

Now that gatekeeper no longer uses it, there are no callers. All tests
have previously been migrated off to avoid potential crosstalk.

This makes the implementation work for rego compilation
sharding/optimization significantly easier.

Signed-off-by: Will Beason willbeason@google.com

Now that gatekeeper no longer uses it, there are no callers. All tests
have previously been migrated off to avoid potential crosstalk. It's
best to wait until we have solid plans for a remote implementation to
consider modifying the design of Client.

This makes the implementation work for reo compilation
sharding/optimization significantly easier.

Signed-off-by: Will Beason <willbeason@google.com>
This is no longer used so it can be safely deleted.

Signed-off-by: Will Beason <willbeason@google.com>
@codecov-commenter
Copy link

Codecov Report

Merging #169 (d65e0d0) into master (eb4524f) will decrease coverage by 0.08%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #169      +/-   ##
==========================================
- Coverage   45.00%   44.92%   -0.09%     
==========================================
  Files          59       59              
  Lines        2795     2776      -19     
==========================================
- Hits         1258     1247      -11     
+ Misses       1295     1286       -9     
- Partials      242      243       +1     
Flag Coverage Δ
unittests 44.92% <ø> (-0.09%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...works/constraint/pkg/client/drivers/local/local.go 65.77% <0.00%> (-2.47%) ⬇️
...y-agent/frameworks/constraint/pkg/client/client.go 69.48% <0.00%> (+1.90%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb4524f...d65e0d0. Read the comment docs.

@willbeason
Copy link
Member Author

Ignore the gatekeeper failure - that is fixed by open-policy-agent/gatekeeper#1762

Copy link
Contributor

@maxsmythe maxsmythe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@sozercan sozercan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ritazh
Copy link
Member

ritazh commented Dec 18, 2021

gatekeeper_test is failing

@willbeason
Copy link
Member Author

gatekeeper_test is failing

This is expected and is fixed by the PR linked in the above comment.

@willbeason willbeason merged commit 4e828a7 into open-policy-agent:master Dec 18, 2021
@willbeason willbeason deleted the no-reset branch December 18, 2021 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants