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

Optimize role manager and add more rbac apis #20

Merged
merged 4 commits into from
Aug 26, 2019

Conversation

xcaptain
Copy link
Contributor

This pull request is originally meant to fix suggestions at #9 but in later commits I added more rbac apis.

1. prefer to use iterator rather than for loop
2. remove unnecessary Rc::clone
because domain can at most be 1 string, so use Option<&str>
rather than Vec<&str>
@xcaptain
Copy link
Contributor Author

@hsluoyz As discussed #9 (comment) I changed domain definition from Vec<&str> to Option<&str> for convenience, PTAL

@codecov
Copy link

codecov bot commented Aug 25, 2019

Codecov Report

Merging #20 into master will increase coverage by 4.78%.
The diff coverage is 84.44%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
+ Coverage   78.01%   82.79%   +4.78%     
==========================================
  Files           4        4              
  Lines         282      279       -3     
==========================================
+ Hits          220      231      +11     
+ Misses         62       48      -14
Impacted Files Coverage Δ
src/rbac/role_manager.rs 100% <ø> (ø) ⬆️
src/adapter/memory_adapter.rs 47.36% <0%> (-2.64%) ⬇️
src/adapter/file_adapter.rs 53.48% <100%> (+1.1%) ⬆️
src/rbac/default_role_manager.rs 91.62% <86.04%> (+6.62%) ⬆️

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 c8f3b25...0e3762e. Read the comment docs.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 26, 2019

lgtm

@hsluoyz hsluoyz merged commit da1372a into casbin:master Aug 26, 2019
@xcaptain xcaptain deleted the feature/optimize-role-manager branch August 26, 2019 05:54
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.

3 participants