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

nginx: add tests for config #715

Merged
merged 1 commit into from
Sep 20, 2024
Merged

Conversation

alxndrsn
Copy link
Contributor

No description provided.

@alxndrsn alxndrsn marked this pull request as ready for review September 19, 2024 16:03
test/test/.gitignore Outdated Show resolved Hide resolved
# KEEP THIS FILE AS CLOSE AS POSSIBLE TO ../nginx.dockerfile
#
# It is ESPECIALLY IMPORTANT to keep IDENTICAL BASE IMAGES.
#
Copy link
Contributor Author

Choose a reason for hiding this comment

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

One alternative to this copy/paste could be INCLUDE+ (https://github.com/edrevo/dockerfile-plus)

Copy link
Member

@lognaturel lognaturel Sep 20, 2024

Choose a reason for hiding this comment

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

Let's address this as part of #677 Then I think we can use our actual published nginx image as the base image. That would include the frontend code which seems fine.

Now I'm confused. Why not use the actual image now? Is it to try to have a more isolated test?

@alxndrsn alxndrsn requested a review from lognaturel September 20, 2024 04:27
@alxndrsn alxndrsn force-pushed the nginx-testing branch 3 times, most recently from b8572ab to 924a107 Compare September 20, 2024 06:21
Copy link
Member

@lognaturel lognaturel 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 that having some dedicated testing of the nginx config is a great idea. I'd like to talk through whether we really need a separate dockerfile for nginx but we can revise later.

@lognaturel lognaturel merged commit 37e1287 into getodk:next Sep 20, 2024
2 checks passed
@alxndrsn alxndrsn deleted the nginx-testing branch October 3, 2024 14:56
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