Skip to content

Conversation

sydneyli
Copy link
Contributor

This will probably conflict with any changes, but the gist of it is to move things and clean up the API abstraction a bit!

api.go, middleware.go, email.go to api/ and util.go to util/-- although util is bare enough that I might move the individual functions to other places.

There's no rush to land this, just something I'm playing around with. Let me know what you think!

@vbrown608
Copy link
Collaborator

I like this direction! I really like how everything in the api package is now getting automatically namespaced. In the past I've wondered if each handler should get its own file in an api package, along with any associated helpers, but that could just be my indoctrination with ruby/rails talking.

What's the logic behind including email.go in that package? It seems a bit different from the rest, which is mostly handlers and code to support them.

@sydneyli
Copy link
Contributor Author

In the past I've wondered if each handler should get its own file in an api package, along with any associated helpers, but that could just be my indoctrination with ruby/rails talking.

There's a lot of different logic in api.go, and I was thinking it does at least make sense to separate the web-request-handling logic from the others.

What's the logic behind including email.go in that package? It seems a bit different from the rest, which is mostly handlers and code to support them.

Good question! I had it in a separate package previously, then decided it really didn't need to be there yet since api was (currently) the only consumer of the email package. I expect that might change, so it might be good to do that pre-emptively.

@sydneyli
Copy link
Contributor Author

I'm still not certain the current separation between email and api is great, but I think it's better than before. Might try and clean it up after!

Copy link
Collaborator

@vbrown608 vbrown608 left a comment

Choose a reason for hiding this comment

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

This is really great!

_ "github.com/joho/godotenv/autoload"
)

func pingHandler(w http.ResponseWriter, r *http.Request) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice to get this out of main.go :)

@sydneyli sydneyli merged commit 7d03f98 into master Jul 1, 2019
@sydneyli sydneyli deleted the cleanup-package-architecture branch July 1, 2019 18:44
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.

2 participants