Skip to content

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