Start carving up RB Gateway into namespaces

Review Request #9767 — Created March 12, 2018 and submitted

brennie
rb-gateway
master
9771
153760f...
rb-gateway

RB Gateway currently only has a single namespace that is getting very
polluted. Because of this, methods that should be private aren't, since
they are accessible within the declared namespace (package main).

This is the first of a series of patches to split this monolithic
package into sub-packages. This patch tackles the repositories
(née repository.go and git_repository.go) package, as well as adds a
helpers package which contains all the test setup code that is used
across package boundaries.

Ideally, this would only have involved the testing infrastructure
changes, but because of how go imports work, we cannot import
github.com/reviewboard/rb-gateway from .../rb-gateway/helpers to
access Repository et al. because rb-gateway is a binary and not a
library.

Ran unit tests.

  • 0
  • 0
  • 1
  • 3
  • 4
Description From Last Updated
brennie
  1. 
      
  2. helpers/repo_helpers.go (Diff revision 1)
     
     
     
     
     
     
     
     
     
     
     
     

    Replace with map literals.

  3. 
      
brennie
david
  1. 
      
  2. routes_test.go (Diff revision 2)
     
     

    This is very strange. Shouldn't we just be importing "helpers"?

    Even better, we should probably put all of these sub-packages into an internal directory.

    1. No, relative imports (it would have to be "./helpers") are not considered good practice. Also, I don't see a benefit to having this as "internal", becuase if people want they can import our packages.

    2. I don't think I want people importing our packages right now. We've made no guarantees or commitments about API stability.

  3. routes_test.go (Diff revision 2)
     
     

    Looks like it would be nice to expose the createTestConfig method that you currently have in util_test.go.

    1. I go through and fix up these tests in a future patch. This was more about replacing checkFatal with assert.Nil etc, I just happened to clean this up a bit.

  4. util_test.go (Diff revision 2)
     
     

    I think it would be a lot cleaner to have the test functions call CreateTestRepo themselves and then pass in repo.Path as a string to this method.

    1. I've fixed this in a subsequent patch.

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

Status: Closed (submitted)

Change Summary:

Pushed to master (599a8b5)
Loading...