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

DOCKER: removed deprecated docker containers and references #1555

Merged
merged 5 commits into from
Dec 12, 2023

Conversation

sirux88
Copy link
Contributor

@sirux88 sirux88 commented Oct 17, 2023

Description

This PR should contain all the necessary steps to remove the deprecated docker container images with its references from the repo.

This is related to the introduction of the new unified/universal docker container image (PR see below)

Aswell this is meant to be WIP until support for these deprecated images will be dropped.

Related Issue

#1369

Type of Change

  • 📚 Examples / docs / tutorials / dependencies update
  • 🔧 Bug fix (non-breaking change which fixes an issue)
  • 🥂 Improvement (non-breaking change which improves an existing feature)
  • 🚀 New feature (non-breaking change which adds functionality)
  • 💥 Breaking change (fix or feature that would cause existing functionality to change)
  • 🔐 Security fix

Checklist

  • I've written tests (if applicable) for all new methods and classes that I created. (rake test)
  • I've added documentation as necessary so users can easily use and understand this feature/fix.

@sirux88
Copy link
Contributor Author

sirux88 commented Oct 17, 2023

Shall we aswell move the content of containers/docker/pwpush one level up to directly into containers/docker @pglombardo?
There is no need to have it in a sub directory any longer.

@MindTooth
Copy link
Contributor

Containters ~= Docker.

I'm where I prefer the Dockerfile in root if it's just building said project.

Then rename containers/ to examples/ or deploy/. 🤷🏻‍♂️

Just to chime in. 😅

@pglombardo
Copy link
Owner

Shall we aswell move the content of containers/docker/pwpush one level up to directly into containers/docker

Sure - that makes sense.

Also I believe I deleted RELEASE.md yesterday - that file is no longer needed.

I like having Dockerfile in the root directory too but we also have the entrypoint.sh, docker compose files and the README so better off in a subdirectory for now.

@pglombardo
Copy link
Owner

Also I pushed 764c0ef to master to update the entrypoint.sh link to documentation. You probably want to update this branch to sync with master when you can.

@sirux88
Copy link
Contributor Author

sirux88 commented Oct 18, 2023

Shall we aswell move the content of containers/docker/pwpush one level up to directly into containers/docker

Sure - that makes sense.

Also I believe I deleted RELEASE.md yesterday - that file is no longer needed.

I like having Dockerfile in the root directory too but we also have the entrypoint.sh, docker compose files and the README so better off in a subdirectory for now.

I agree. Lets keep it in this sub dir for now.

@sirux88 sirux88 force-pushed the remove-deprecated-containers branch from e38f23e to 1a9d619 Compare October 18, 2023 17:28
@sirux88
Copy link
Contributor Author

sirux88 commented Oct 18, 2023

Also I pushed 764c0ef to master to update the entrypoint.sh link to documentation. You probably want to update this branch to sync with master when you can.

I'll try to keep this draft in sync to master until support for the obsolete containers finally will be dropped

@sirux88
Copy link
Contributor Author

sirux88 commented Oct 18, 2023

@pglombardo
I also scanned the wiki for necessary changes concering this draft.
As far as I can tell it's only this paragraph that is outdated.

But since it is not directly related to this PR it can be updated anyway.
If it's ok for you I'll update it by myself

@pglombardo
Copy link
Owner

pglombardo commented Oct 19, 2023

Shall we aswell move the content of containers/docker/pwpush one level up to directly into containers/docker

I like having Dockerfile in the root directory too but we also have the entrypoint.sh, docker compose files and the README so better off in a subdirectory for now.

This was in response to @MindTooth. @sirux88 If you want to move the files up to containers/docker that makes sense to me.

Now I see you already did - perfect.

I also scanned the wiki for necessary changes concering this draft.
As far as I can tell it's only this [paragraph](https://github.com/pglombardo/PasswordPusher/wiki/Switch-to-Another-> > > Backend-Database#overview) that is outdated.

But since it is not directly related to this PR it can be updated anyway.
If it's ok for you I'll update it by myself

By all means, feel free/thanks.

@sirux88
Copy link
Contributor Author

sirux88 commented Oct 21, 2023

This was in response to @MindTooth. @sirux88 If you want to move the files up to containers/docker that makes sense to me.

Now I see you already did - perfect.

What I wrote was a bit confusing. I just wanted to agree to your suggestion.

I also scanned the wiki for necessary changes concering this draft.
As far as I can tell it's only this [paragraph](https://github.com/pglombardo/PasswordPusher/wiki/Switch-to-Another-> > > Backend-Database#overview) that is outdated.
But since it is not directly related to this PR it can be updated anyway.
If it's ok for you I'll update it by myself

By all means, feel free/thanks.

Done.

@sirux88 sirux88 force-pushed the remove-deprecated-containers branch from 1a9d619 to d527bfa Compare November 17, 2023 15:18
@pglombardo pglombardo added the dependencies Pull requests that update a dependency file label Dec 5, 2023
@pglombardo
Copy link
Owner

Hey @sirux88 - whenever you can let's merge this. ...time to clean up and get rid of the old stuff.

@sirux88 sirux88 force-pushed the remove-deprecated-containers branch from d527bfa to bbbccbc Compare December 9, 2023 09:47
@sirux88
Copy link
Contributor Author

sirux88 commented Dec 9, 2023

@pglombardo: I rebased to current master. Aswell I rescanned for any obsolete references but didn't find any.

So this should be ready for merging

@sirux88 sirux88 marked this pull request as ready for review December 9, 2023 09:49
@sirux88
Copy link
Contributor Author

sirux88 commented Dec 9, 2023

@pglombardo: Maybe we should change the depracation notice like this one for the old images into a end of support notice build, let the images be built and pushed to docker hub.

Let me know if you want to do this. I'll throw another PR then.

Afterwards we could merge this PR.

@pglombardo
Copy link
Owner

Ah good point - that's a great idea. Let's do that by all means.

@sirux88 sirux88 force-pushed the remove-deprecated-containers branch from bbbccbc to 7b103fd Compare December 12, 2023 19:15
@sirux88
Copy link
Contributor Author

sirux88 commented Dec 12, 2023

should be ready for merge now @pglombardo

@pglombardo pglombardo merged commit f8c3a25 into pglombardo:master Dec 12, 2023
4 checks passed
@pglombardo
Copy link
Owner

And with that merge, it concludes the final move to the universal container. Thanks to both of you for the input and contributions to get to this point!

bprobst-msis pushed a commit to MarketScan/PasswordPusher that referenced this pull request Dec 29, 2023
…do#1555)

* removed obsolete docker containers and references

* Move container files up a level and fix references

* fix some text references

* fix dependabot action

* fix out-of-context links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants