Validate commit #105
Validate commit #105
Conversation
dpordomingo
left a comment
There was a problem hiding this comment.
LGTM
But I'd require only one thing: to separate each feature in a dedicated commit. That way the changes will be easily tracked. Once you do it, you can auto merge :)
Makefile
Outdated
| # simple go get doesn't work for client go | ||
| install-client-go: | ||
| go get gopkg.in/bblfsh/client-go.v2 | ||
| go get -d gopkg.in/bblfsh/client-go.v2 |
There was a problem hiding this comment.
Also a "-v -u" will be helpfull
|
@dpordomingo all features are related to each other. For CI step we need to keep assets unchanged for the same code, modtime doesn't help much without disabling source code mapping and we can't disable mapping without updating react-scripts. So the point is there is no much profit in any of this changes without new CI step and this PR is about it. Only .dockerignore doesn't belong here, but it's such a small change, I think we don't really need separate PR for it. |
- Use modtime 1 for go-bindata to avoid changes of assets.go - Don't copy node_modules inside docker container - Disable source map for production build to avoid assets differences - Updated react-scripts to 1.0.17 - Validate assets.go as CI step Signed-off-by: smacker <max@smacker.ru>
Signed-off-by: Maxim Sukharev <maxim@sourced.tech>
bzz
left a comment
There was a problem hiding this comment.
CI failure is not relevant to the changes and is tracked under https://github.com/bblfsh/client-go/issues/50
Is there anything left before merging this one?
|
@bzz I don't think it's network failure. It compiles correctly locally on my machine (without cache), on Denis machine and in travis on |
|
This https://github.com/bblfsh/client-go/pull/51 should fix the issue with Travis |
|
@dpordomingo thank you! Fix seems to be merged now, shall we update something here as part of this PR to benefit from it? |
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
Signed-off-by: David Pordomingo <David.Pordomingo.F@gmail.com>
|
ci is green. I'm merging. |
Signed-off-by: smacker max@smacker.ru