-
Notifications
You must be signed in to change notification settings - Fork 262
Infra File Cleanup #5596
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
Infra File Cleanup #5596
Conversation
Does not appear to be used anywhere.
Doing as a single commit in case of ressurection.
cddc677 to
aae5494
Compare
| # Really seems like it _should_ be here, as it's referenced by `nginx.conf`. | ||
| # But it's hasn't been for years. | ||
| # COPY ${SRC_DIR}/mime.types /etc/nginx/mime.types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odd thing I found 🤷
…ndard. And deleting defunct cloudbuild prod yaml
Making a separate commit for bookmarking purposes. Seems we used to deploy to a separate/distinct dev cluster, and create new postgres instances for every single pr.
Standardizing on image-specific docker locations.
Will delete after both have been updated.
0de1409 to
0c39da8
Compare
|
It doesn't look it, but I did make sure to register file movements, renames, etc with git. In GH, the file history looks incomplete. But if you |
645e5a5 to
1394aba
Compare
rtibbles
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading through, I didn't see anything that alarmed me here, and checking out locally, the dev server still seems to work!
| skip_after_successful_duplicate: false | ||
| github_token: ${{ github.token }} | ||
| paths: '["k8s/images/nginx/*", ".github/workflows/containerbuild.yml"]' | ||
| paths: '["docker/Dockerfile.nginx.prod", "nginx/*", ".github/workflows/containerbuild.yml"]' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the second one "nginx/*" perhaps should have been "docker/nginx/*"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I… think maybe your right. Either that, or it was supposed to look at k8s/images/nginx/*, for the sake of the CD pipeline 🧐
The paths here were shuffled around multiple times and I think I got lost somewhere in that flurry 🙃
But this is the commit where it happened… so. idk. DXCanas@1dbd0db
Should I submit another PR to correct this? is it breaking build?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rtibbles opened a followup yesterday! It's fixed
Summary
Cleaning up infra-related files that no longer appear to be in use. Tried to bundle mass deletions by subject so we can go back for history.
Of Historical Note:
References
#5520
https://github.com/learningequality/infrastructure/issues/853
Reviewer guidance
It might be clearer to go commit-by-commit rather than seeing the full file changelog.
Really, I'm asking for y'all to make sure there are no breaking changes for the local dev workflow (I will deal with consequences to infra-side)