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

The template no longer use the node image in production #213

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

sindre-nistad
Copy link
Contributor

regression from 3c6ebd9

Why is this pull request needed?

#206 changed how the images were built, by removing the node / web image in production and replacing it with NginX.
I didn't make the necessary changes in .github/workflows/publish-image.yaml to reflect this change.

What does this pull request change?

Makes sure the previous web component is removed / renamed in favour of the nginx component.
In principle, the logic for creating the web image was moved to the nginx image. In practice, the web image was renamed, and adjusted slightly, while the old nginx image was removed.

Issues related to this change:

Copy link
Contributor

@jorgenengelsen jorgenengelsen left a comment

Choose a reason for hiding this comment

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

Not sure if I prefer NGIX naming over WEB, I think WEB is clearer. But it looks like this will fix the issue so approved by me

@sindre-nistad
Copy link
Contributor Author

Not sure if I prefer NGIX naming over WEB, I think WEB is clearer. But it looks like this will fix the issue so approved by me

The reason I chose nginx is because I assume it is used as the entry point (the container with exposed ports) to the app when deployed, and I didn't want to make changes there as well.

@sindre-nistad sindre-nistad merged commit 5ee2211 into main Jun 26, 2023
@sindre-nistad sindre-nistad deleted the hotfix/node-image-was-removed-for-prod branch June 26, 2023 08:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants