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

Update casbin-orm-adapter model name #3

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

conghuhu
Copy link

@conghuhu conghuhu commented Aug 7, 2022

I think it's better to modify this piece to be consistent with the examples

Fix: #2

@CLAassistant
Copy link

CLAassistant commented Aug 7, 2022

CLA assistant check
All committers have signed the CLA.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 7, 2022

@conghuhu plz fix:

image

@hsluoyz hsluoyz requested a review from rushitote August 7, 2022 16:25
@hsluoyz
Copy link
Member

hsluoyz commented Aug 7, 2022

@rushitote plz review

Copy link
Member

@rushitote rushitote left a comment

Choose a reason for hiding this comment

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

I am not sure if we should do it this way, because then the module will need to be imported by require("CasbinAdapter") - which is too general and doesn't specify what kind of adapter is this.

Instead we could rename the file to CasbinORMAdapter.lua for consistency, what do you think?

@conghuhu
Copy link
Author

conghuhu commented Aug 8, 2022

OK, then I will continue to use CasbinORMAdapter

@hsluoyz
Copy link
Member

hsluoyz commented Aug 8, 2022

@rushitote if this is the case, actually we have another ORM adapter: https://github.com/casbin-lua/luasql-adapter , so CasbinORMAdapter is perhaps not a good name either. What about 4DaysOrmAdapter or 4DaysORMAdapter?

  • Can we remove the Casbin prefix?
  • ORM or Orm?

@rushitote
Copy link
Member

@hsluoyz
Yes, 4DaysOrmAdapter seems great and I think we can keep it Orm since it's a common acronym.

@hsluoyz
Copy link
Member

hsluoyz commented Aug 8, 2022

@conghuhu plz use this name: 4DaysOrmAdapter

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.

moude name 'CasbinORMAdapter' is different from examples 'CasbinAdapter'
4 participants