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

Added more detailed instructions on how to setup the repo #1114

Merged
merged 3 commits into from Aug 14, 2017

Conversation

Projects
None yet
5 participants
@crmejia
Contributor

crmejia commented Aug 9, 2017

This change adds instructions to setup the repo locally. On my first experience with the code base I lost a lot of time running a single test through docker. I was unable to run go test ./... because I had naively cloned my fork. So I did some research and found the proper way to set up the repo in the k8s dev guide So I decided to add those instructions to help people jumping in the code.

PS. I copied most of the words from those instructions and referenced the guide. Is this OK or do I need to paraphrase?

git clone https://github.com/$user/service-catalog.git
# or: git clone git@github.com:$user/service-catalog.git
cd $working_dir/service-catalog

This comment has been minimized.

@duglin

duglin Aug 9, 2017

Contributor

nit: technically it could just be cd service-catalog

The Service Catalog github repository can be found
[here](https://github.com/kubernetes-incubator/service-catalog.git).
### 1 Fork in the Cloud
1. Visit Visit https://github.com/kubernetes-incubator/service-catalog

This comment has been minimized.

@duglin

duglin Aug 9, 2017

Contributor

s/Visit Visit/Visit/

@@ -88,14 +88,50 @@ also need:
a Kubernetes cluster. As such, our build process only supports compilation of
linux/amd64 binaries suitable for execution within a Docker container.

## Cloning the Repo
## Workflow
We can set up the repo by following a process similar to the [dev guide for k8s]( https://github.com/kubernetes/community/blob/master/contributors/devel/development.md#1-fork-in-the-cloud)

This comment has been minimized.

@duglin

duglin Aug 9, 2017

Contributor

it'd be nice to wrap this at 80 cols.

This comment has been minimized.

@crmejia

crmejia Aug 10, 2017

Contributor

I think this one wraps at exactly 80 but the markup doesn't


$ git clone https://github.com/kubernetes-incubator/service-catalog.git
Per Go's workspace instructions, place Service Catalog's code on your GOPATH using the following cloning procedure.

This comment has been minimized.

@duglin

duglin Aug 9, 2017

Contributor

wrap at 80 please

@duglin

This comment has been minimized.

Contributor

duglin commented Aug 9, 2017

CI failure looks real:
./docs/devguide.md: Can't find section '#cloning-the-repo' in ./docs/devguide.md

Overall, it SGTM, just a few nits.

crmejia added some commits Aug 10, 2017

@crmejia

This comment has been minimized.

Contributor

crmejia commented Aug 10, 2017

Fixed the issues.

@duglin Is it OK that I copied part of the k8s dev guide?

@duglin

This comment has been minimized.

Contributor

duglin commented Aug 10, 2017

I don't see why not, but if it reads ok just having a pointer to the kube docs (so we don't duplicate or get our of sync) would be good too.

@crmejia

This comment has been minimized.

Contributor

crmejia commented Aug 13, 2017

I see your point and agree it works either way. Also, I think it won't be out of sync since these are standard git workflow instructions, so having them there saves a click.

@arschles

LGTM. Thanks @crmejia for this contribution!

@arschles arschles added the LGTM1 label Aug 14, 2017

@vaikas-google vaikas-google added the LGTM2 label Aug 14, 2017

@vaikas-google vaikas-google merged commit 4e642d5 into kubernetes-incubator:master Aug 14, 2017

3 checks passed

cla/linuxfoundation crmejia authorized
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
service-catalog-PR-testing2 Successful presubmits. Details at https://service-catalog-jenkins.appspot.com/job/service-catalog-PR-testing2/1760/.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment