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

Create dockerfile #47

Merged
merged 18 commits into from
Nov 28, 2023
Merged

Create dockerfile #47

merged 18 commits into from
Nov 28, 2023

Conversation

Ughuuu
Copy link

@Ughuuu Ughuuu commented Nov 26, 2023

📓 Description

Adds docker build to this and exports the docker to GitHub packages so people can run it easier.

☑️ Checklist

  • Documentation is up to date
  • [?] Versions are bumped as needed

Updated the README

🔗 Related issues

#46

@Ughuuu Ughuuu marked this pull request as draft November 26, 2023 11:21
@Ughuuu
Copy link
Author

Ughuuu commented Nov 26, 2023

Will test it here: https://github.com/Ughuuu/noray/actions
As it seems that here I can't run the build without approval.

@Ughuuu
Copy link
Author

Ughuuu commented Nov 26, 2023

Ok, works and it generates a package of this form: https://github.com/Ughuuu/noray/pkgs/container/noray
docker pull ghcr.io/ughuuu/noray:pr-1

For this repo it will be docker pull ghcr.io/foxssake/noray:pr-1
Note that if using without a tag it will try to pull :latest I think, so probably you will want to use :main after main builds one docker.
But no way of testing it without merging to main first I think.

Copy link

@elementbound elementbound left a comment

Choose a reason for hiding this comment

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

Did a review, some comments might not be relevant with the PR being a draft, but let me know!

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
.github/workflows/ci.yml Outdated Show resolved Hide resolved
@Ughuuu Ughuuu marked this pull request as ready for review November 26, 2023 12:03
@Ughuuu
Copy link
Author

Ughuuu commented Nov 26, 2023

Updated feedback. Locally seems ok, looks like it should work but you never know really unless you merge and it runs on main branch.(Changed that it only pushes on main branch docker and builds on both pr and main)
This is the latest run: https://github.com/Ughuuu/noray/actions/runs/6995432654

You can enable the workflows to run also here to double check.

@Ughuuu
Copy link
Author

Ughuuu commented Nov 26, 2023

Saw this happen also locally where e2e tests would hang, but I assumed it was from my frok of the repo.

@Ughuuu
Copy link
Author

Ughuuu commented Nov 27, 2023

Can you rerun the e2e test please some times and see if it passes? Usually it should pass in 20s or so.

Copy link

@elementbound elementbound left a comment

Choose a reason for hiding this comment

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

Hi, had some time to go through the PR in more detail. Added some minor comments, but I'm also happy to do the fixups if you invite me to the fork.

Dockerfile Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
.github/workflows/docker-publish.yml Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
@elementbound
Copy link

The e2e step seems to be struggling, let me look into it.

@elementbound
Copy link

FYI this will be a "chore" commit as it doesn't affect the main application logic. Please bump the patch component in package.json.

Copy link

@elementbound elementbound left a comment

Choose a reason for hiding this comment

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

LGTM, thank you for the feature and patience during the review process!

@elementbound elementbound merged commit 822deca into foxssake:main Nov 28, 2023
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.

2 participants