Skip to content

Conversation

@Wynnfan
Copy link
Member

@Wynnfan Wynnfan commented Sep 20, 2018

Provide method can modify the table name.

@coveralls
Copy link

coveralls commented Sep 20, 2018

Coverage Status

Coverage decreased (-2.3%) to 83.684% when pulling 597cf36 on Wynnfan:mongodb-adapter-wy into f79831b on casbin:master.

Copy link
Member

@hsluoyz hsluoyz left a comment

Choose a reason for hiding this comment

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

Please fix my comments.

Moreover, please merge your commits into one.

adapter.go Outdated
}

db := session.DB(dI.Database)
collection := db.C(name)
Copy link
Member

Choose a reason for hiding this comment

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

The name here should be "casbin_rule", because it's the table name, not the db name.

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, the name really is the table name. I found write the wrong notes when i push,but i have modified,so have three commit.

Copy link
Member

Choose a reason for hiding this comment

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

So you should squash your commits into one in this PR. Because there's no need to see how you fix the errors in the commit history.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thank you,I am a rookie. About this name modify, is it still necessary to submit?

Copy link
Member

@hsluoyz hsluoyz Sep 20, 2018

Choose a reason for hiding this comment

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

Please join the Tencent QQ group for discussion: 546057381

adapter.go Outdated
}

// Can use provided database name.
func NewDBAdapter(url, name string) persist.Adapter {
Copy link
Member

Choose a reason for hiding this comment

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

Please do not create another constructor method. Just do it as how we did it in Xorm Adapter: https://github.com/casbin/xorm-adapter/blob/6d86407ce2631ca1f6f6943374c6129c5b03e190/adapter.go#L50-L75

@Wynnfan Wynnfan closed this Sep 20, 2018
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