for me is a must upload the vendoring

don't see much profit from it. With proposed flow you have to compile frontend after go-get anyway. Running dep ensure there doesn't make it worse.
But not committing it allows using the tool with other frontend or whatever another crazy use case when people want to have their own vendoring.
Correct me if I'm wrong but before if your dependency has vendor directory you could have a conflict of dependencies.

Still if everybody wants to have vendor in repo, it's not such a big deal for me.

As I said is standard in go.

@campoy you were a go evangelist, right? Can you give me more information about it, please? All I read on the internet, for example, https://golang.github.io/dep/docs/FAQ.html#should-i-commit-my-vendor-directory says it's up to project.

This says everything https://github.com/golang/dep/tree/master/vendor

That's a great question, actually. So good it's part of the FAQ of dep.

From the docs:

Should I commit my vendor directory?

It's up to you:

Pros

It's the only way to get truly reproducible builds, as it guards against upstream renames, deletes and commit history overwrites.
You don't need an extra dep ensure step to sync vendor/ with Gopkg.lock after most operations, such as go get, cloning, getting latest, merging, etc.

Cons

Your repo will be bigger, potentially a lot bigger, though prune can help minimize this problem.
PR diffs will include changes for files under vendor/ when Gopkg.lock is modified, however files in vendor/ are hidden by default on Github.

So I'll let @mcuadros decide, since he's the CTO. I do think we should commit vendored dependencies, since this allows for anyone to simply do go get of our repo and everything will simply work.

the problem is it wouldn't just work.
but no problem. I don't really have a strong opinion on it.
Just wanted some guideline for my projects outside of src-d.

As I wrote, almost every single relevant project is committing the vendor. So please commit the vendor.

release build should be packages something like:

packages: build-frontend  dependencies-backend bindata packages docker-build

Also, you need to force the order of the dependencies with rule: | one two three otherwise if someone use -j n will fail

This information should go int he CONTRIBUTING.md this is not the place for this.

yes. We have #29 for it.
I'll rebase later.

actually I should just remove it

You should copy the file from the bin folder like:
https://github.com/src-d/ci/blob/v1/examples/basic/Dockerfile#L2

And then you can remove almost the full Dockefile

this way allows to build docker file on any machine without installing any dependencies like go or nodejs. It was requirement from @vmarkovtsev to be able to build docker file without correct version of nodejs and yarn

Yes, I can confirm that: I even have no idea what is yarn and why it is better than npm

Err but if you are contributing you need to run the tests and other stuff. So you need yarn installed anyway.

Anyways you need to call make packages for having the proper version variables.

If contributing - agree 100%. If building the container to run the thing and forget about it - nopes.

don't mess up CI and release with local building of container.

example use case: I'm a devops. I want to run deploy this tool on my cluster. There is a change in master I need. But no release yet. So I can just git clone && docker build -t my/tool . && docker push my/tool && kubectl <deploy>

Errr, no. You shouldn't, never do this, you should our standard image, tagged and tests. This Dockerfile is for the CI to build docker images.

But I am not understanding this discussion. We agreed and merged a CI, to be used, and not to ignore it two days after.

The goal of CI is exactly this one, having homogeneous repositories.

okay, will remove build step from Dockerfile. I'm just used to be able to build images without external dependencies.

P.S. example of our app (merged by you):
https://github.com/src-d/github-reminder/blob/master/Dockerfile

you should include .ci

.ci is there:
https://github.com/src-d/code-annotation/pull/36/files/09f1a6ced524c250859222d93f9916b280c9b5ba#diff-a084b794bc0759e7a6b77810e01874f2R27

# ci files
Makefile.main
.ci
bin

The binary should be in the PATH and as an ENTRYPOINT and not as a CMD

https://docs.docker.com/engine/userguide/eng-image/dockerfile_best-practices/#entryopoint

Also, you need to force the order of the dependencies with rule: | one two three otherwise if someone use -j n will fail

This link works? Or just links to a folder called releases

will fix

You should put a standard path to place the binary.

Something like:

wget https://github.com/mcuadros/passage/releases/download/v0.1.0/passage_v0.1.0_linux_amd64.tar.gz
tar -xvzf passage_v0.1.0_linux_amd64.tar.gz
cp passage_v0.1.0_linux_amd64/passage /usr/local/bin/

agreed. But I want to postpone it until we have our first release.

this is a good idea? maybe is better return a 404 if is file not found and a 500 if not

it is. Frontend has urls like /experiment/1/2, backend knows nothing about them. It's handled correctly if you click on a link (js will catch event), but if you open this url the first time response will be served by the server. That's why we have to return index.html. JS there will handle routing correctly (and show 404 if path doesn't exist). It's a limitation for single page application if you want to have nice urls without #.

meeeh

frontend 😂

for your platform

leftover?

why? now we don't do anything besides copying binary into docker. So we don't need to send node_modules (it can be huge and takes time)

he is maybe asking about the extra / ??

I find extremely odd having a .env file and not very devops friendly. If we are going ot have it, at least include the definition of each variable in the templete. But I prefer having it explained in the readme and having defaults.

I think the .env file is just an easy way to have in one place all the environmental values as expected by the app, but it is up to the developers to decide how they are providing them;
Environment values can be passed as regular vars (ie.UI_DOMAIN=http://127.0.0.1:3000 make gorun) or as regular docker envs (ie. docker run --env OAUTH_CLIENT_ID=id --env OAUTH_CLIENT_SECRET=secret --rm -p 8080:8080 srcd/code-annotation)

Most of them are documented in different parts of the README.md or the CONTRIBUTING.md.

@mcuadros Should we block this PR till everything is properly documented, or can we merge and handle this requirement in a separated issue?

These are the environment values recognized by the app;
most of them are not mandatory, but we can also make "optional" the JWT_SIGNING_KEY.

.env file is just a simple way to see/edit environmental variables for local development. Most of the values have defaults. All variables must and will be documented. But this PR isn't the right place to do that.

About devops:
In my experience env vars are the most friendly and natural way to configure an application with kubernetes, thanks to configmap,secrets objects and valueFrom in container spec.

And The twelve-factor app also recommends it.

@dpordomingo thank you for great summary!

As on @mcuadros notice, agreed that we should have it clearly documented - have added it to the scope of #67

@smacker where says that is recommended have a .env file? This is not recommended at all, and almost none use this.

Also is bad not having any prefix on your env vars.

If you are speaking about the usage of env vars, of course, is recommended, and actually is a MUST that all source{d} applications

@mcuadros no-no. I mean env vars, not .env file. We use .env file ONLY for local development. Agreed about prefix, we should add it to env vars without prefix! Thanks!

Didn't know it's must, https://github.com/bblfsh/dashboard uses flags instead. But glad to hear.