-
Notifications
You must be signed in to change notification settings - Fork 4
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
Refactor: Move away from a forked model #1
Conversation
We are moving away from a forked model to a packaged helm dependency model for helm service. As a result, the helm artifacts are removed altogether, and the helm dependency is included in go.mod. The project is renamed to albatross and the dependency path source files is updated.
Using GOMODULES=on/auto during go get updates the go.mod file which is non ideal. Setting GOMODULES=off and installing golangci-lint with go get fails.
cmd/albatross/albatross.go
Outdated
@@ -6,10 +6,11 @@ import ( | |||
|
|||
"github.com/gorilla/mux" | |||
|
|||
"github.com/gojekfarm/albatross/pkg/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.
we'll end up changing this to github.com/gojek
again later.
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.
Ack
.travis.yml
Outdated
[ ! -z "$TRAVIS_COMMIT" ] && \ | ||
docker tag gojektech/albatross:latest gojektech/albatross:$TRAVIS_COMMIT && \ | ||
docker push gojektech/albatross:$TRAVIS_COMMIT | ||
|
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.
let's remove trailing and ending whitespaces & newline
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
build: | ||
@echo "Building './bin/albatross'" | ||
@mkdir -p ./bin | ||
@go build -o bin/albatross ./cmd/albatross |
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.
call the package as cmd/server
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 one concern I can think of with renaming the package as server is that go install with default options will result in a binary named service. We will be using Makefile/Dockerfile to build the binary for workflows, so that shouldn't matter much, but it might be slighly unintuitive I feel. Looked at other go repos as well, and they seem to be following a similar convention.
"github.com/gojekfarm/albatross/pkg/api" | ||
"github.com/gojekfarm/albatross/pkg/api/logger" | ||
"github.com/gojekfarm/albatross/pkg/servercontext" | ||
|
||
"helm.sh/helm/v3/pkg/action" |
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.
let's remove pkg
folder, and move servercontext
to config
package
This PR refactors the codebase to move away from a forked model. Instead, helm is introduced as an external dependency.
As part of this refactor, additional resources are added to support a standalone repo: