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

Added support for gorm/v2 #36

Merged
merged 3 commits into from
Jul 22, 2020
Merged

Added support for gorm/v2 #36

merged 3 commits into from
Jul 22, 2020

Conversation

L1ghtman2k
Copy link
Contributor

gorm v2 has been released on gorm.io/gorm
Documentation is currently under http://v2.gorm.io/

This pull request aims to make gorm-adapter work with gorm/v2

Signed-off-by: l1ghtman2k <aibek.zhil@gmail.com>
@coveralls
Copy link

coveralls commented Jul 21, 2020

Coverage Status

Coverage increased (+0.5%) to 63.11% when pulling 9e04eef on L1ghtman2k:master into 9ebbe7e on casbin:master.

@L1ghtman2k
Copy link
Contributor Author

Please review the versioning change I made in the go.mod file, as I am not sure how exactly the versioning supposes to happen in this case. (In this pull request, I made it github.com/casbin/gorm-adapter/v3)

@@ -9,7 +9,7 @@ Gorm Adapter
[![Gitter](https://badges.gitter.im/Join%20Chat.svg)](https://gitter.im/casbin/lobby)
[![Sourcegraph](https://sourcegraph.com/github.com/casbin/gorm-adapter/-/badge.svg)](https://sourcegraph.com/github.com/casbin/gorm-adapter?badge)

Gorm Adapter is the [Gorm](https://github.com/jinzhu/gorm) adapter for [Casbin](https://github.com/casbin/casbin). With this library, Casbin can load policy from Gorm supported database or save policy to it.
Gorm Adapter is the [Gorm](https://gorm.io/gorm) adapter for [Casbin](https://github.com/casbin/casbin). With this library, Casbin can load policy from Gorm supported database or save policy to it.
Copy link
Member

Choose a reason for hiding this comment

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

You should update import path for gorm-adapter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed with c780b42

adapter.go Outdated
@@ -94,9 +88,6 @@ func NewAdapter(driverName string, dataSourceName string, dbSpecified ...bool) (
return nil, err
}

// Call the destructor when the object is released.
runtime.SetFinalizer(a, finalizer)
Copy link
Member

Choose a reason for hiding this comment

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

Why remove it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I did it at the time due to gorm changing the default closure statement for the database(I assumed gorm/v2 did not need a Close() call). I believe I misinterpreted it, and after reading go-gorm/gorm#3145, I have returned a modified finalizer(9e04eef).

Signed-off-by: l1ghtman2k <aibek.zhil@gmail.com>
Signed-off-by: l1ghtman2k <aibek.zhil@gmail.com>
@nodece nodece merged commit 57a0c40 into casbin:master Jul 22, 2020
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

3 participants