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

Change update policy workflow #28

Merged
merged 2 commits into from
Jun 12, 2020
Merged

Change update policy workflow #28

merged 2 commits into from
Jun 12, 2020

Conversation

sagilio
Copy link
Member

@sagilio sagilio commented Jun 11, 2020

close #19

@hsluoyz
Copy link
Member

hsluoyz commented Jun 11, 2020

@sagilio which is better for Boolean v.s. bool? and String v.s. string? Please use another PR to fix all styles, not this one.

@hsluoyz
Copy link
Member

hsluoyz commented Jun 11, 2020

@huazhikui @xcaptain @nodece please review.

@hsluoyz
Copy link
Member

hsluoyz commented Jun 11, 2020

@sagilio the AppVeyor seems to failed. Can you make a PR to fix that first? So we can have good-to-go CI in this PR: https://ci.appveyor.com/project/hsluoyz/casbin-net/builds/33452140

@sagilio
Copy link
Member Author

sagilio commented Jun 11, 2020

@sagilio the AppVeyor seems to failed. Can you make a PR to fix that first? So we can have good-to-go CI in this PR: https://ci.appveyor.com/project/hsluoyz/casbin-net/builds/33452140

I try to run this build at my fork repository, It has not this error. So I think it may this ci option caused.

image

@hsluoyz
Copy link
Member

hsluoyz commented Jun 11, 2020

I try to run this build at my fork repository, It has not this error. So I think it may this ci option caused.

@sagilio so how to fix it? What value should I change it to?

@hsluoyz
Copy link
Member

hsluoyz commented Jun 11, 2020

@sagilio please resolve conflicts.

@hsluoyz
Copy link
Member

hsluoyz commented Jun 11, 2020

We should also clarify one thing: if an adapter doesn't support Auto-Save, then calling enforcer's add/remove policy will only modify the local model, since DB side will never be updated.

BTW, I think we should put these into official docs.

@hsluoyz
Copy link
Member

hsluoyz commented Jun 12, 2020

Approved.

What do you think? @xcaptain @nodece

@nodece
Copy link
Member

nodece commented Jun 12, 2020

@hsluoyz LGTM

@hsluoyz hsluoyz merged commit 72c8e1a into casbin:master Jun 12, 2020
@hsluoyz
Copy link
Member

hsluoyz commented Jun 12, 2020

@huazhikui please release a new version

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.

No rollback of memory data when adapter throw exception
4 participants