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.

Sign up for GitHub

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

Jump to bottom

feat: add sample for migrating efcore adapter. #51

Merged
merged 1 commit into from
Sep 25, 2022
Merged

feat: add sample for migrating efcore adapter. #51

merged 1 commit into from
Sep 25, 2022

Conversation

AsakusaRinne
Copy link
Contributor

Add a sample for migrating efcore-adapter with casbin-aspnetcore. #41

Currently, the dependency is set as local directory. I will change it after the corresponding PR in efcore-adapter is merged.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 21, 2022

@AsakusaRinne plz fix:

image

@AsakusaRinne
Copy link
Contributor Author

@AsakusaRinne plz fix:

image

It failed because the efcore-adapter has not been updated, though I could build it successfully in my local environment. I'll revise it after the update of efcore-adapter.

1 similar comment
@AsakusaRinne
Copy link
Contributor Author

@AsakusaRinne plz fix:

image

It failed because the efcore-adapter has not been updated, though I could build it successfully in my local environment. I'll revise it after the update of efcore-adapter.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 22, 2022

@AsakusaRinne can you update the efcore-adapter?

@AsakusaRinne
Copy link
Contributor Author

@AsakusaRinne can you update the efcore-adapter?

I've submitted a PR to efcore-adapter and I'll keep communication with sagilio to revise it until it's good enough.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 22, 2022

@AsakusaRinne any link?

@AsakusaRinne
Copy link
Contributor Author

@AsakusaRinne any link?

Please refer to this PR.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 22, 2022

@AsakusaRinne merged: casbin-net/efcore-adapter#63

@sagilio
Copy link
Member

sagilio commented Sep 10, 2022

@AsakusaRinne plz fix now

@hsluoyz
Copy link
Member

hsluoyz commented Sep 11, 2022

@AsakusaRinne plz fix:

image

@AsakusaRinne
Copy link
Contributor Author

@AsakusaRinne plz fix:

image

Now it has been fixed. Thanks for your help and sagilio.

@hsluoyz
Copy link
Member

hsluoyz commented Sep 13, 2022

@sagilio

sagilio
sagilio approved these changes Sep 23, 2022
View reviewed changes
Copy link
Member

@sagilio sagilio left a comment

Choose a reason for hiding this comment

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

image
LGTM, it is best to fix this warning

@AsakusaRinne
Copy link
Contributor Author

image LGTM, it is best to fix this warning

OK, done.

@hsluoyz
Copy link
Member

hsluoyz commented Sep 25, 2022

@AsakusaRinne fix all warnings:

image

/cc @sagilio

@AsakusaRinne
Copy link
Contributor Author

@AsakusaRinne fix all warnings:

image

/cc @sagilio

It's not a warning caused by this PR. The warning exists in src folder, while this PR only add the code of a sample.

@hsluoyz hsluoyz merged commit 164e56f into casbin-net:preview Sep 25, 2022
@github-actions
Copy link

github-actions bot commented Sep 1, 2023

🎉 This PR is included in version 1.0.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sagilio sagilio sagilio approved these changes

Assignees
No one assigned
Labels
released
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@AsakusaRinne @hsluoyz @sagilio