Skip to content

Conversation

@rishabhpoddar
Copy link
Contributor

@rishabhpoddar rishabhpoddar commented Jun 9, 2021

@Piyushhbhutoria I have made several changes:

  • Removed use of interface and replaced them with struct
  • Added proper error handling for all functions
  • Fixed a bunch of "known" bugs (from my previous code review)
  • Remove storing of instance inside structs to simulate singleton behavior
  • Made APIKey a pointer to a string in querier
  • Some functions that were supposed to be static were actually instance functions, fixed that
  • Fixed normalization of emailverification recipe
  • Used fixed returns types for recipe and API functions (as opposed to generic map interface)
  • Added otherHandler var to APIOption (not sure if this is the right thing to do, but it's what's done in node)
  • Using MakeStruct function pattern to create new structs.
  • Removed unnecessary use of pointers in certain places

I can get on a call to discuss all of the above. There are also a few TODOs I have added that I would like to discuss with you.

NOTE: There will be compile time errors in the session recipe due to some of the changes to recipeModule etc..

HandleAPIRequest func(id string, req *http.Request, w http.ResponseWriter, path NormalisedURLPath, method string) error
GetAllCORSHeaders func() []string
GetAPIsHandled func() []APIHandled
GetAPIsHandled func() (*[]APIHandled, error)
Copy link
Contributor

Choose a reason for hiding this comment

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

no need of pointer. slice can be nil.

func MakeRecipeModule(recipeId string, appInfo NormalisedAppinfo,
HandleAPIRequest func(id string, req *http.Request, w http.ResponseWriter, path NormalisedURLPath, method string) error,
GetAllCORSHeaders func() []string,
GetAPIsHandled func() (*[]APIHandled, error)) RecipeModule {
Copy link
Contributor

Choose a reason for hiding this comment

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

no need of pointer. slice can be nil.

@Piyushhbhutoria Piyushhbhutoria merged commit c84dbb5 into 0.0 Jun 10, 2021
nkshah2 added a commit that referenced this pull request Apr 14, 2023
…n-functions

refactor: Remove deprecated session functions (JWT Rework #7)
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