-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use Standard Go program layout #37
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be a good idea to separate the API from the service logic and repository layers. This is a well-known architectural pattern called Hexagonal Architecture https://dev.to/bagashiz/building-restful-api-with-hexagonal-architecture-in-go-1mij
.gitignore
Outdated
@@ -1,2 +1,2 @@ | |||
whitepointinventory | |||
build | |||
.idea/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add new line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
build/build_run.sh
Outdated
|
||
go build -o build/whitepointinventory ./cmd/whitepointinventory | ||
|
||
build/whitepointinventory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add new line
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
@@ -0,0 +1,111 @@ | |||
package main | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is one binary project you can use cmd/main.go
rather than cmd/whitepointinventory/whitepointinventory.go
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
} | ||
log.Printf("Server starting on port %s", portString) | ||
|
||
err = srv.ListenAndServe() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
inline error check
if err := srv.ListenAndServer(); err != nil {
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implemented
docker/Dockerfile
Outdated
@@ -10,7 +10,7 @@ RUN go mod download | |||
|
|||
COPY . . | |||
|
|||
RUN go build -o whitepointinventory | |||
RUN go build -o whitepointinventory ./cmd/whitepointinventory | |||
|
|||
FROM alpine:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use scratch instead of an alpine image
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed to a scratch base image
docker/Dockerfile
Outdated
@@ -10,7 +10,7 @@ RUN go mod download | |||
|
|||
COPY . . | |||
|
|||
RUN go build -o whitepointinventory | |||
RUN go build -o whitepointinventory ./cmd/whitepointinventory | |||
|
|||
FROM alpine:latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't copy the env file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed.
@@ -0,0 +1,5 @@ | |||
rm -rf build/whitepointinventory |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It will overwrite no need to remove
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The script runs the old binary if there's an error in compilation, which is undesired behaviour for this particular use case. Hence why I remove it.
pkg/jsonresponses/json.go
Outdated
@@ -1,12 +1,12 @@ | |||
package main | |||
package jsonresponses |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a good package name you can rename it to internal/api
-> package api
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I intend the APIs here to be public APIs. What would be a good package name to still have them in the pkg directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Public you mean people outside the project would import it as a package?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The change the name
pkg/health/health.go
Outdated
"net/http" | ||
) | ||
|
||
func HandlerHealth(w http.ResponseWriter, r *http.Request) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Have a look at https://github.com/mszostok/version or https://github.com/nelkinda/health-go then you can create a health endpoint with those values
This is the standard https://datatracker.ietf.org/doc/html/draft-inadarei-api-health-check#section-5
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
…explicitly defined in docker-compose.yml file
Covers cases where .env is not needed e.g. when environment is explicitly defined in a docker-compose.yml file
Had to refactor entire code-base. I am doing some clean up and then you can re-review. You can just check of all your comments are handled. |
02a8287
to
38c35d7
Compare
Done. |
You can check if your comments are handled now. |
|
||
func (a *UserAuth) MiddlewareAuth(handler authedHandler) http.HandlerFunc { | ||
return func(w http.ResponseWriter, r *http.Request) { | ||
APIKey, err := auth.GetAPIKey(r.Header) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APIKey, err := auth.GetAPIKey(r.Header) | |
apiKey, err := auth.GetAPIKey(r.Header) |
For non exported variables start with small letter. This applies to everyplace.
@@ -0,0 +1,33 @@ | |||
package middleware |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
rename file to auth.go
@@ -0,0 +1,11 @@ | |||
package domain |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try and reduce the underscores in file naming. Use DDD where we have different domains farmer, payments, purchases and users
domain(for example users)
api
repositoryrepository.go
middleware(if you have any)
domain(for example user).go (defines the repository and service interface)
service.go (implements service)
# GNU General Public License for more details. | ||
# | ||
# You should have received a copy of the GNU General Public License | ||
# along with this program. If not, see <http://www.gnu.org/licenses/>. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add this to CI for example https://github.com/rodneyosodo/gophercon-africa-2024/blob/main/.github/workflows/ci.yaml
Guided by project-layout