[GitHub] [yunikorn-k8shim] wilfred-s commented on pull request #626: [YUNIKORN-1844] PreEnqueue plugin implementation

["wilfred-s (via GitHub)" <gi...@apache.org>]

wilfred-s commented on PR #626:
URL: https://github.com/apache/yunikorn-k8shim/pull/626#issuecomment-1614055936

   Code looks good. I was wondering if we can refactor the code a bit and move the event setup. `EventsToRegister` seems a bit out of place in the context and in the predicate manager just for the plugin callback. 
   Should the scheduler plugin itself also move into the `plugin` package. instead of being its own package?
   
   One comment on the code: instead of passing around `types.UID` cast it to a string and pass the string, it is always a string for us.


-- 
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: reviews-unsubscribe@yunikorn.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org