Skip to content
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

tools: fix Dockerfile #1769

Closed
wants to merge 1 commit into from
Closed

tools: fix Dockerfile #1769

wants to merge 1 commit into from

Conversation

resmo
Copy link
Member

@resmo resmo commented Nov 22, 2016

Build a docker image did not work anymore for various reasons. This is how I fixed it

@rohityadavcloud
Copy link
Member

LGTM. @pdion891 would you like to review?

@pdion891
Copy link
Contributor

Could you look if it conflict with #1435 ? Because I made a pull request to automatically increment version in docker file when changing release of master via tools/build/setnextversion.sh.

@wido
Copy link
Contributor

wido commented Nov 25, 2016

Changes look good to me, but waiting for @pdion891 to tell us more :)

@resmo
Copy link
Member Author

resmo commented Nov 25, 2016

I didn't test #1435 yet, but code looks good. The only difference I see is:

pip install cryptography --force-reinstall

And I used the mysql connector from MySQL.com but would prefer to take it from distribution (as in #1435)

@pdion891
Copy link
Contributor

So, should we go forward and merge #1435 + #1769 to master?
I might just need to rebase and update version number of the current commit.

With those 2 PR, we would have master back online on dockerhub so we can get an updater docker image for the simulator!

@pdion891
Copy link
Contributor

@resmo could you do a PR on top of #1435 ? or this have to be merged first? so we can get your chances and some of mine.

@resmo
Copy link
Member Author

resmo commented Nov 30, 2016

@pdion891 I would say merge #1435 first

@pdion891
Copy link
Contributor

ok, I'll update the PR and merge it I think the PR go all required LGTM. I'll confirmed with John, in case he is in a code freeze period.

@jburwell jburwell mentioned this pull request Dec 1, 2016
@rohityadavcloud
Copy link
Member

@resmo now that PR by @pdion891 is merge, can this be closed? Thanks.

@pdion891
Copy link
Contributor

pdion891 commented Dec 9, 2016

Hi @resmo, I've made some changes on the Docker file so it might not be possible to merge this PR in the current state.

Thanks!

@resmo
Copy link
Member Author

resmo commented Dec 9, 2016

ok, np closing for now

@resmo resmo closed this Dec 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants