roryqi commented on PR #10968:
URL: https://github.com/apache/gravitino/pull/10968#issuecomment-4387532442

   > > > > I don't think this is a good module name. Because it will make 
others confused. We have the authenticator class in the server-common. I don't 
think that idp feature is suitable to call the module `authenticators/basic`. 
@jerryshao WDYT?
   > > > 
   > > > 
   > > > What is your suggestion?
   > > 
   > > 
   > > I prefer using the module name `idp`. We can put most of this feature 
code into the module `idp`. We can use only put `BasicAuthenticator` into the 
module `server-common`.
   > 
   > `idp` covers too many things, we'd better narrow down the scopes, how 
about `idp-basic`?
   
   It will be weird that first level directory has a name called idp. Idp isn't 
an important function. I prefer using `plugin/idp`. It will better to make it 
as the second level directories.
   
   Agree with your opinion to narrow down the scope.
   `idp-basic` seems adjective. `basic-idp` seems better for me
   
   Based above all, `plugin/basic-idp` would be better from me. WDYT? 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to