Remove reliance on global state

Review Request #9782 — Created March 15, 2018 and submitted

Information

rb-gateway
master
d9ade07...

Reviewers

This patch is a cleanup of the API routes. The major change is that we
now have an API type which contains the API methods. With this change,
the routes now have access to the API's configuration so we no longer
need a global Config object!

Additionally, some cleanup has been done to the routes that require a
repository with the api.withRepository middleware, which provides the
requested repository as the "repo" context variable (instead of having
each route retrieve it and do error checking). Routes have been
re-written to have a simpler error-checking flow as well.

Ran unit tests.

Description From Last Updated

Do we need to fetch the config from out of this package? It seems better if this were unexported.

daviddavid

Wrapping here doesn't really improve readability.

daviddavid

I think I prefer having the table of path/method/handler that we iterate over to build all the routes.

daviddavid

This is not a good error message, plus things should use log rather than println. Leftover debug output?

daviddavid

Same here.

daviddavid

Leftover debug output?

daviddavid

Typo: Contenet

daviddavid

This needs to be run through gofmt

daviddavid
brennie
david
  1. 
      
  2. api/api.go (Diff revision 2)
     
     

    Do we need to fetch the config from out of this package? It seems better if this were unexported.

    1. I was using it as a way to set it, but I guess we can unexport it and have a setter?

  3. api/api.go (Diff revision 2)
     
     
     

    Wrapping here doesn't really improve readability.

  4. api/api.go (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     

    I think I prefer having the table of path/method/handler that we iterate over to build all the routes.

  5. api/api.go (Diff revision 2)
     
     

    This is not a good error message, plus things should use log rather than println. Leftover debug output?

  6. api/api.go (Diff revision 2)
     
     

    Same here.

  7. api/routes.go (Diff revision 2)
     
     

    Leftover debug output?

  8. api/routes.go (Diff revision 2)
     
     

    Typo: Contenet

  9. 
      
brennie
david
  1. 
      
  2. api/api.go (Diff revision 3)
     
     
     
     
     
     
     
     
     
     
     
     

    This needs to be run through gofmt

  3. 
      
brennie
brennie
david
  1. Ship It!
  2. 
      
brennie
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to master (6c77911)
Loading...