Skip to content
This repository was archived by the owner on Jun 14, 2023. It is now read-only.

Conversation

@lumost
Copy link
Contributor

@lumost lumost commented Feb 24, 2015

@eng-group-ops @Tapjoy/eng-group-tools @Tapjoy/dynamiq

I've updated the deploy/build script to correct a bug where we could only ever slugforge build master.

the new code will support slugforge building a branch

deploy/build Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

if the branch is master, will this fail?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

On Tue, Feb 24, 2015 at 11:53 AM, StabbyCutyou [email protected]
wrote:

In deploy/build
#69 (comment):

@@ -6,7 +6,12 @@ set -e
mkdir -p build
cd build
export GOPATH=pwd
-go get github.com/Tapjoy/dynamiq
+export GITBRANCH=git branch | grep '*' | grep -Eo '[^ ]*$'
+go get -d github.com/Tapjoy/dynamiq
+cd src/github.com/Tapjoy/dynamiq
+git checkout -b $GITBRANCH

if the branch is master, will this fail?


Reply to this email directly or view it on GitHub
https://github.com/Tapjoy/dynamiq/pull/69/files#r25268888.

@StabbyCutyou
Copy link
Contributor

LGTM but i am in no way the authority on how to write these deploy scripts

@ehealy
Copy link

ehealy commented Apr 27, 2015

@lumost, @csmith0651 and I were looking at this and wanted to touch base. based on the configuration of the slug-builder job on Jenkins (http://jenkins.tapjoy.net/job/slug-builder/configure), it looks like Jenkins is taking care of putting you on a clean copy of the correct branch to build and clears out any modifications that may have been made by a previous build. Unless there is some reaon I'm missing that we need to create the build directory and take the actions outlined in this script, we can probably simplify this considerably. Lets touch base and discuss.

Choose a reason for hiding this comment

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

Use pushd and popd if you're going to do this:

pushd src/github.com/Tapjoy/dynamiq
# do stuff
popd # now back at ../../../..

@jlogsdon
Copy link

Yeah, jenkins should handle fetching the repo (and the specified branch) and make sure its in a clean state. The build script should simply run the commands required to build prepare it for slugging. If the branch parameter is being ignored that may be an issue with the setup in Jenkins.

@ehealy
Copy link

ehealy commented Apr 27, 2015

@lumost @csmith0651 and I discussed this and this makes sense since go is building a binary and managing the fetching of dependent git repos internally as opposed to using something like bundler when compiling. The pushd/popd improvement @jlogsdon suggested is a good idea to implement here.

@ehealy ehealy closed this Apr 27, 2015
@ehealy ehealy reopened this Apr 27, 2015
@lumost
Copy link
Contributor Author

lumost commented Apr 27, 2015

@jlogsdon

This is how the go toolchain works, golang understands remote import
statements such as

import ( "flag" "github.com/Sirupsen/logrus" "github.com/Tapjoy/dynamiq/app"
)

when you "go get" a repo it pulls down all the dependencies for that repo
for you ( it understands when to git clone/svn co etc. ) and then compiles
a binary.

unfortunately it only understands how to fetch master - so we need to cd
into the dir which it has checked out and switch to the branch that we want.

anouther strategy used is to check the dependencies into you're own repo
such as in https://github.com/etsy/Hound/tree/0.1

this is generally a bad idea and hound switched away from that strategy for
v0.2

--JAmes

On Mon, Apr 27, 2015 at 11:17 AM, ehealy [email protected] wrote:

Reopened #69 #69.


Reply to this email directly or view it on GitHub
#69 (comment).

@jlogsdon
Copy link

unfortunately it only understands how to fetch master

Really? That seems like an odd design decision... Can you lock in the
version of a dep?
On Apr 27, 2015 11:33 AM, "James Moore" [email protected] wrote:

@jlogsdon

This is how the go toolchain works, golang understands remote import
statements such as

import ( "flag" "github.com/Sirupsen/logrus" "
github.com/Tapjoy/dynamiq/app"
)

when you "go get" a repo it pulls down all the dependencies for that repo
for you ( it understands when to git clone/svn co etc. ) and then compiles
a binary.

unfortunately it only understands how to fetch master - so we need to cd
into the dir which it has checked out and switch to the branch that we
want.

anouther strategy used is to check the dependencies into you're own repo
such as in https://github.com/etsy/Hound/tree/0.1

this is generally a bad idea and hound switched away from that strategy for
v0.2

--JAmes

On Mon, Apr 27, 2015 at 11:17 AM, ehealy [email protected] wrote:

Reopened #69 #69.


Reply to this email directly or view it on GitHub
#69 (comment).


Reply to this email directly or view it on GitHub
#69 (comment).

@StabbyCutyou
Copy link
Contributor

Hahahahahahhahahahahahahahahahahahhahahahahahahahahahahahahahhahahaha

no

@StabbyCutyou
Copy link
Contributor

But don't worry, the wizards of Go are hard at work at a vendoring solution. It involved REWRITING YOUR IMPORT STATEMENTS FOR YOU instead of just adding a vendor path to the lookup chain prior to the original location for the dependencies to live.

Because golang

@lumost
Copy link
Contributor Author

lumost commented Apr 27, 2015

stabby, google believes that you sent a message in filipino.

but to the original question, go makes decisions you do not fight the
decisions. there are a couple vendoring setups out there, and golang is
going to add native vendoring support to the toolchain later on.

--James

2015-04-27 11:38 GMT-04:00 StabbyCutyou [email protected]:

Hahahahahahhahahahahahahahahahahahhahahahahahahahahahahahahahhahahaha

no


Reply to this email directly or view it on GitHub
#69 (comment).

@jlogsdon
Copy link

Cripes. I thought go had a good toolchain. For google I'm sure its fine
with how they work, but LOL at expecting master to always work for you in
OSS
On Apr 27, 2015 11:39 AM, "StabbyCutyou" [email protected] wrote:

But don't worry, the wizards of Go are hard at work at a vendoring
solution. It involved REWRITING YOUR IMPORT STATEMENTS FOR YOU instead of
just adding a vendor path to the lookup chain prior to the original
location for the dependencies to live.

Because golang


Reply to this email directly or view it on GitHub
#69 (comment).

@lumost
Copy link
Contributor Author

lumost commented Apr 27, 2015

I still don't understand why I can't import "github.com/tapjoy/lane#version"

--James

On Mon, Apr 27, 2015 at 11:39 AM, StabbyCutyou [email protected]
wrote:

But don't worry, the wizards of Go are hard at work at a vendoring
solution. It involved REWRITING YOUR IMPORT STATEMENTS FOR YOU instead of
just adding a vendor path to the lookup chain prior to the original
location for the dependencies to live.

Because golang


Reply to this email directly or view it on GitHub
#69 (comment).

@lumost
Copy link
Contributor Author

lumost commented Apr 27, 2015

oddly we still haven't actually encountered a problem with master in 8
months of golang.

On Mon, Apr 27, 2015 at 11:41 AM, James Moore [email protected]
wrote:

I still don't understand why I can't import "
github.com/tapjoy/lane#version"

--James

On Mon, Apr 27, 2015 at 11:39 AM, StabbyCutyou [email protected]
wrote:

But don't worry, the wizards of Go are hard at work at a vendoring
solution. It involved REWRITING YOUR IMPORT STATEMENTS FOR YOU instead of
just adding a vendor path to the lookup chain prior to the original
location for the dependencies to live.

Because golang


Reply to this email directly or view it on GitHub
#69 (comment).

@StabbyCutyou
Copy link
Contributor

+1 to not fighting with go. It works well, even if you hate how it works, so long as you just let it.

@lumost I also don't get it. You could claim that versioning can be tough, not everything is semver, so comparing versions to find a "later" one is not always cut and dry. They could mandate semver, but instead they'd probably invent "gover" and have it work 85% like semver, but with 3 catastrophic twists.

@StabbyCutyou
Copy link
Contributor

Gusto ko na pag-ibig " golang ", ngunit ang mga may-akda gumawa ng mahirap

Hows that for filipino

@jlogsdon
Copy link

Just use git shas 😏
On Apr 27, 2015 11:44 AM, "StabbyCutyou" [email protected] wrote:

+1 to not fighting with go. It works well, even if you hate how it works,
so long as you just let it.

@lumost https://github.com/lumost I also don't get it. You could claim
that versioning can be tough, not everything is semver, so comparing
versions to find a "later" one is not always cut and dry. They could
mandate semver, but instead they'd probably invent "gover" and have it work
85% like semver, but with 3 catastrophic twists.


Reply to this email directly or view it on GitHub
#69 (comment).

@lumost
Copy link
Contributor Author

lumost commented Apr 27, 2015

go get supports non-git,

although looking at the docs for vcs, it looks like its possible to pass
flags into the checkout command.....
https://godoc.org/golang.org/x/tools/go/vcs

On Mon, Apr 27, 2015 at 11:48 AM, James Logsdon [email protected]
wrote:

Just use git shas [image: 😏]
On Apr 27, 2015 11:44 AM, "StabbyCutyou" [email protected] wrote:

+1 to not fighting with go. It works well, even if you hate how it works,
so long as you just let it.

@lumost https://github.com/lumost I also don't get it. You could claim
that versioning can be tough, not everything is semver, so comparing
versions to find a "later" one is not always cut and dry. They could
mandate semver, but instead they'd probably invent "gover" and have it
work
85% like semver, but with 3 catastrophic twists.


Reply to this email directly or view it on GitHub
#69 (comment).


Reply to this email directly or view it on GitHub
#69 (comment).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants