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

Ci tools documentation updates #12

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

AugustasV
Copy link
Contributor

@AugustasV AugustasV commented Nov 28, 2021

Added GitHub action that would push the image into the docker hub. Right now it is configured to trigger only the master branch. Configuration is made into my docker hub account, probably would be better to switch to Github Packages?
Also wrote documentation on how to spin up local Kubernetes cluster and test things locally using DevSpace. Small README improvements.

Looking for a feedback on how to improve this PR.
Not possible to add as a reviewer, so tagging like that @pijuszczyk @pawsten

Copy link
Collaborator

@pawsten pawsten left a comment

Choose a reason for hiding this comment

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

Thank you for your PR but I'm not sure whether It's a good idea to bind so tightly build system with minikube on docker container driver and devspace which we don't use. Isn't it overengineering?

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@pijuszczyk pijuszczyk left a comment

Choose a reason for hiding this comment

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

I agree with Paweł. We shouldn't bind the builds to minikube and devspace.
Regarding pushing images, I'll contact the relevant people and let you know how it could be solved.

.github/workflows/c-cpp.yml Outdated Show resolved Hide resolved
devspace.yaml Outdated Show resolved Hide resolved
@AugustasV
Copy link
Contributor Author

AugustasV commented Nov 30, 2021

Probably I didn't explained properly. Minikube and devspace are only used for local development and testing purposes, imitating Kubernetes cluster. Github actions building, tagging, and pushing image into docker hub.

@AugustasV AugustasV requested a review from pijuszczyk January 11, 2022 07:16
@pijuszczyk
Copy link
Contributor

Hi @AugustasV ,
First of all, sorry for taking so long. Unfortunately the topic of pushing images is still in progress and I can't point you to any solution at the moment. To go forward with this pull request, could you currently limit the scope of changes to building on Github and stash changes that require image pushing & fetching for later?

That could be achieved by sticking to the changes in c-cpp.yml (except logging in to Docker Hub and push: true) and saving other changes for later. I'll try to establish how we should handle images as soon as possible.

@AugustasV
Copy link
Contributor Author

AugustasV commented Jan 17, 2022

Sure @pijuszczyk I could do that, or maybe we should go different approach? I could use Github Packages to save Docker images here on Github. It's free for public repos https://github.com/features/packages#pricing

@pijuszczyk
Copy link
Contributor

Thanks for the suggestion, we could use that. However, still first I'd need to discuss this topic. I think a conclusion could be reached sometime next week. If you don't mind waiting a little longer I'll reach you then about what can be done in this case. But if you prefer to close the pull request sooner, I'd recommend stashing the changes like I described above.

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.

3 participants