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

ROX-22044: postgresql 15 #1416

Merged
merged 1 commit into from
Mar 15, 2024
Merged

ROX-22044: postgresql 15 #1416

merged 1 commit into from
Mar 15, 2024

Conversation

RTann
Copy link
Collaborator

@RTann RTann commented Feb 24, 2024

See for related changes: stackrox/rox-ci-image#198

DB integration tests run in a GitHub Workflow, so it should be using PostgreSQL 15. If not, I also tested it locally by running make db-integration-tests while running a PostgreSQL 15 server in the background.

@RTann RTann force-pushed the ross/pg-15 branch 3 times, most recently from 169c0f0 to 84dd6ac Compare March 13, 2024 17:36
@RTann RTann changed the title chore(db): postgresql 15 ROX-22044: postgresql 15 Mar 13, 2024
@BradLugo
Copy link
Contributor

BradLugo commented Mar 13, 2024

I wouldn't normally comment on this, but since we're changing it in a few places, we don't generally favor printf over echo AFAIK. As for the readability (or even perhaps the functionality) of string interpolation, we usually use bash's parameter expansion syntax, i.e., ${MYVAR}. That being said, I wouldn't block a PR on this nit

@RTann
Copy link
Collaborator Author

RTann commented Mar 13, 2024

I wouldn't normally comment on this, but since we're changing it in a few places, we don't generally favor printf over echo AFAIK. As for the readability (or even perhaps the functionality) of string interpolation, we usually use bash's parameter expansion syntax, i.e., ${MYVAR}. That being said, I wouldn't block a PR on this nit

I took these changes directly from https://github.com/docker-library/postgres/blob/44ef8b226a40f86cf9df3f9299067db6779a3aa3/15/bookworm/docker-entrypoint.sh and I think it'd be best to stay as close to the original source as possible, so I'm going to keep it as-is

Copy link
Contributor

@BradLugo BradLugo left a comment

Choose a reason for hiding this comment

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

Barring the image tags, LGTM!

Copy link

openshift-ci bot commented Mar 15, 2024

@RTann: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name Commit Details Required Rerun command
ci/prow/slim-e2e-tests 435e979 link false /test slim-e2e-tests
ci/prow/e2e-tests 435e979 link false /test e2e-tests

Full PR test history. Your PR dashboard.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

@RTann RTann merged commit 0ef4177 into master Mar 15, 2024
16 of 18 checks passed
@RTann RTann deleted the ross/pg-15 branch March 15, 2024 17:24
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