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

feat: pre-build dockerfile #201

Merged
merged 5 commits into from Dec 5, 2022
Merged

feat: pre-build dockerfile #201

merged 5 commits into from Dec 5, 2022

Conversation

sammcj
Copy link
Collaborator

@sammcj sammcj commented Oct 6, 2022

Summary of changes

Fix for #163

  • Build, label, tag and publish Docker image to Githubs image repository.
  • Simplify Action's Dockerfile to pull the built image.

Do any of the followings changes break current behaviour or configuration?

  • No

How changes have been tested

  • I've forked the repo and done the build and publish.
  • Tested I'm able to pull the built image locally.

List any unknowns

  • I haven't tested the changes in this repo.
    • I don't know what package permissions @anothrNick has setup on this repo, if it's default I believe public repos have public packages.

Once merged, the Action should be tested to ensure it's operating the same.

@sammcj sammcj self-assigned this Oct 6, 2022
@sammcj sammcj requested a review from sbe-arg October 6, 2022 06:15
@sammcj
Copy link
Collaborator Author

sammcj commented Oct 6, 2022

Note @sbe-arg the build will never work for this PR as it's running the build workflow from main - rather than the branch.

(One of the many limitations of Github Actions is that you can't trigger a workflow run on a PR that creates the workflow - it has to be merged into main first 😞)

@sammcj sammcj marked this pull request as ready for review October 6, 2022 07:13
@sammcj sammcj requested a review from anothrNick October 6, 2022 21:05
@sbe-arg
Copy link
Collaborator

sbe-arg commented Oct 7, 2022

Can we test this in a fork and show proof then we merge master against master.
Is not urgent? Ill try to test during the weekend.

@sbe-arg
Copy link
Collaborator

sbe-arg commented Oct 7, 2022

But at first sight I like it.

@sammcj
Copy link
Collaborator Author

sammcj commented Dec 5, 2022

@sbe-arg note I believe the test-action is only failing to pull the image because - it doesn't exist yet 😄

Copy link
Collaborator

@sbe-arg sbe-arg left a comment

Choose a reason for hiding this comment

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

Merge and ping me if anything goes wrong but looks good

@sammcj sammcj merged commit 60b1f44 into anothrNick:master Dec 5, 2022
@sammcj
Copy link
Collaborator Author

sammcj commented Dec 5, 2022

Thanks, will do and I'll just revert it right away, no point in stuffing around etc... 😄

@sammcj
Copy link
Collaborator Author

sammcj commented Dec 5, 2022

aww 😢 looking into why the post-merge build failed

@sammcj
Copy link
Collaborator Author

sammcj commented Dec 5, 2022

follow up PR up #219
if that doesn't work, I'll revert.

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

2 participants