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.

Sign up for GitHub

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

Jump to bottom

Feat: Dockerize tokei #930

Merged
merged 6 commits into from
Jul 5, 2023
Merged

Feat: Dockerize tokei #930

merged 6 commits into from
Jul 5, 2023

Conversation

mihaigalos
Copy link
Contributor

@mihaigalos mihaigalos commented Jul 22, 2022 •
edited
Loading

This PR contributes an Earthfile as well as instructions on how to self-build a dockerized version of tokei.

There was a previous attempt to dockerize tokei in PR #297.
That resolves to a separate repo and the resulting image is >20 MB big.

This PR's image is about 5 MB (latest), smaller because it is based on alpine and using multi-stage docker builds.

@mihaigalos
Copy link
Contributor Author

mihaigalos commented Jul 22, 2022 •
edited
Loading

I can also help with a CD pipeline to automatically build and deploy to Docker Hub, like here.
Let me know if you would need it.

@mihaigalos mihaigalos changed the title Dockerize tokei Feat: Dockerize tokei Jul 22, 2022
@mihaigalos
Copy link
Contributor Author

Hi @XAMPPRocky, the changes in this are not affecting any compiled or tested files.
I had a look why the CI is failing. Seems the Actions are failing because of an unrelated error.

@XAMPPRocky
Copy link
Owner

Thank you for your PR! Yeah the stuff seems unrelated. This PR looks good, but I'm thinking about using https://earthly.dev for CI/CD so I'm not sure if I'll accept this (if not I'll add you as a co-author to the commit)

@mihaigalos
Copy link
Contributor Author

mihaigalos commented Jul 27, 2022 •
edited
Loading

That's ok. I knew about earthly, but never used it. Now I'm curious! 😊

@XAMPPRocky
Copy link
Owner

If you'd like to learn/try it, I would accept this if it was changed to earthly.

@mihaigalos
Copy link
Contributor Author

Yes, I would love to contribute, actually!
Do you already have work for the CI pipeline for earthly?

@XAMPPRocky
Copy link
Owner

XAMPPRocky commented Jul 27, 2022 •
edited
Loading

Not in this project no. I think to start with we would just have this as an extra step to the end and just ensure that the image builds and works, rather than trying to move everything to earthly.

@mihaigalos
Copy link
Contributor Author

I can add a CI job if you want. Let me know.

@mihaigalos mihaigalos marked this pull request as draft July 27, 2022 17:32
@mihaigalos mihaigalos marked this pull request as ready for review July 27, 2022 17:48
@mihaigalos
Copy link
Contributor Author

Ok, works.
If this gets merged I can concentrate on a separate PR for the CI pipeline.

XAMPPRocky
XAMPPRocky reviewed Jul 27, 2022
View reviewed changes
Earthfile Outdated Show resolved Hide resolved
@mihaigalos @XAMPPRocky
Remove superfluous referencing of build step
fa91e89 
This is not needed because it is implied.

Co-authored-by: XAMPPRocky <4464295+XAMPPRocky@users.noreply.github.com>
@mihaigalos
Copy link
Contributor Author

Hi. Any updates here?

@XAMPPRocky
Copy link
Owner

Hey, I’m on holidays so it will be reviewed after.

@mihaigalos
Copy link
Contributor Author

Ok, enjoy and let's talk again when you're back! 😄

@mihaigalos
Copy link
Contributor Author

Up.

1 similar comment
@PMExtra
Copy link

PMExtra commented Jul 5, 2023

Up.

@XAMPPRocky
Copy link
Owner

Sorry for the delay, I'm going to merge this in assuming the failure was spurious from the platform, but I'll revert if it fails mainline, and would ask that someone opens a new PR with whatever required fixes.

Thank you for your PR, and congrats on your first contribution! 🎉

@XAMPPRocky XAMPPRocky merged commit d5e820a into XAMPPRocky:master Jul 5, 2023
@mihaigalos mihaigalos deleted the dockerize_tokei branch July 5, 2023 19:30
@github-actions github-actions bot mentioned this pull request Mar 4, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@XAMPPRocky XAMPPRocky XAMPPRocky left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mihaigalos @XAMPPRocky @PMExtra