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

remove dependence on and references to db docker definition in delphi/operations repo #1000

Merged
merged 6 commits into from
Dec 6, 2022

Conversation

melange396
Copy link
Collaborator

  • removes building of delphi_database from ci.yaml and dev Makefile
  • updates related documentation
  • also annotates dev/docker/python/Dockerfile with a reference to its dependency on delphi/operations repository

we havent needed dev/docker/database/Dockerfile from delphi/operations (or the delphi_database image it builds) since the v4 merge, which moved to building on a Percona container.

also some documentation updates
we havent needed `dev/docker/database/Dockerfile` from `delphi/operations` since the v4 merge, which moved to building on a Percona container
@melange396 melange396 added documentation dependencies Pull requests that update a dependency file code health readability, maintainability, best practices, etc devops building, running, deploying, environment stuff, handy utils, repository-related, engineer QoL, etc labels Oct 10, 2022
dshemetov
dshemetov previously approved these changes Oct 10, 2022
Copy link
Contributor

@dshemetov dshemetov left a comment

Choose a reason for hiding this comment

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

lgtm, happy to see our dependencies and Dockerfiles slowly get unified, instead of being split across multiple repos

@@ -78,7 +78,7 @@ web:
@# Build the web_epidata image
@cd repos/delphi/delphi-epidata;\
docker build -t delphi_web_epidata -f ./devops/Dockerfile .;\
cd ../../../
cd -
Copy link
Contributor

Choose a reason for hiding this comment

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

til: lifehack! productivity +100% 👍

@dshemetov
Copy link
Contributor

tag #965

@melange396
Copy link
Collaborator Author

nice, didnt realize you had that issue open regarding the operations repo [in actuality, it was probably less "didnt realize" and more "forgot"]

@melange396 melange396 changed the base branch from main to dev October 10, 2022 20:33
@melange396 melange396 dismissed dshemetov’s stale review October 10, 2022 20:33

The base branch was changed.

@melange396 melange396 changed the base branch from dev to main October 10, 2022 20:34
@melange396 melange396 changed the base branch from main to dev October 10, 2022 20:42
also some documentation updates
we havent needed `dev/docker/database/Dockerfile` from `delphi/operations` since the v4 merge, which moved to building on a Percona container
…/delphi-epidata into rm_operations_database_docker
@melange396
Copy link
Collaborator Author

ugh, i made a mess of the commit history, but this should now be against the dev branch instead of main. :(

dshemetov
dshemetov previously approved these changes Oct 10, 2022
Copy link
Contributor

@dshemetov dshemetov left a comment

Choose a reason for hiding this comment

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

diffs look the same as before, approving again

@dshemetov
Copy link
Contributor

you can rebase, clean, and force push, if you want. i didn't check this PR locally

krivard
krivard previously approved these changes Oct 14, 2022
Copy link
Contributor

@krivard krivard left a comment

Choose a reason for hiding this comment

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

👍 j'approve

@melange396
Copy link
Collaborator Author

whoops, i let this languish for a month and a half, gonna merge now

@melange396 melange396 dismissed stale reviews from krivard and dshemetov via e32fd2f December 6, 2022 14:43
@melange396
Copy link
Collaborator Author

...or rather, i will merge when i get another approval

@melange396
Copy link
Collaborator Author

this also furthers the goals of #965

@melange396
Copy link
Collaborator Author

fwiw, when i fixed the merge conflicts on this, i went with the dev branch for all of them

@melange396 melange396 merged commit 9d97fe7 into dev Dec 6, 2022
@melange396 melange396 deleted the rm_operations_database_docker branch December 6, 2022 19:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
code health readability, maintainability, best practices, etc dependencies Pull requests that update a dependency file devops building, running, deploying, environment stuff, handy utils, repository-related, engineer QoL, etc documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants