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

Changed docs to differentiate between docker compose standalone and plugin #22394

Merged
merged 2 commits into from
Feb 7, 2023

Conversation

JulianRommel
Copy link
Contributor

@JulianRommel JulianRommel commented Feb 5, 2023

What & How

I changed the local deployment instructions to better differentiate between docker-compose (standalone version) and docker compose (plugin). The docs aren't concise on this currently. I also changed the link to the Docker Engine installation guide, as this includes the Docker Compose plugin. Changed "tested on Ubuntu 20.04" to "... Ubuntu 22.04" and fixed typo on line 54(thanks @juweins).

🚨 User Impact 🚨

Should help users who haven't used docker before to find the correct version.

@CLAassistant
Copy link

CLAassistant commented Feb 5, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ JulianRommel
❌ Julian Rommel


Julian Rommel seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

@octavia-squidington-iii octavia-squidington-iii added area/documentation Improvements or additions to documentation community labels Feb 5, 2023
Copy link
Contributor

@juweins juweins left a comment

Choose a reason for hiding this comment

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

Hey @JulianRommel ! Thank you for your first Contribution! Welcome 🎉

I reviewed your PR and I have got two questions for you:

  1. Regarding your proposal: Is there really a need for instructions on docker engine (standalone)?

I would say no, since this adds complexity to the quick start and may cause some confusion (docker compose vs. -) Furthermore, user of the standalone version will be aware of using the (deprecated) hyphen command.

  1. I found a typo on line 54: there is a backslash \ following the WSL_ which causes it to display in the documentation. It would be great if you can include this in you PR.

  2. Please make sure to sign the CLA, so your PR can be further processed.

Thoughts?

@JulianRommel
Copy link
Contributor Author

Hey @juweins and thank you so much for your feedback! You are probably right about the standalone version. I will change it and also include a fix for the typo on line 54.

Removed the docker compose standalone instructions, changed "tested on Ubuntu" version number and fixed typo
Copy link
Contributor

@juweins juweins left a comment

Choose a reason for hiding this comment

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

Thanks for removing the odd character in line 54.

From my side, this LGTM! The changes are reasonable and provide value. @sh4sh

BTW:
My previous comment should initiate a little discussion and not terminate your proposals. You can bring in your thoughts, too! 👍🏽

Make sure to sign the CLA from your previous commit. I don't know what happened here.

@JulianRommel
Copy link
Contributor Author

@juweins that's weird, I have signed the CLA but it seems to be buggy. Any ideas how to fix this? I already removed it and signed it again...

Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

Thanks @JulianRommel

@marcosmarxm marcosmarxm enabled auto-merge (squash) February 7, 2023 15:01
@marcosmarxm marcosmarxm merged commit e441d5c into airbytehq:master Feb 7, 2023
danidelvalle pushed a commit to danidelvalle/airbyte that referenced this pull request Feb 9, 2023
…lugin (airbytehq#22394)

* Changed local deployment instructions to differentiate between docker compose plugin and standalone version

* Removed docker compose standalone version

Removed the docker compose standalone instructions, changed "tested on Ubuntu" version number and fixed typo

---------

Co-authored-by: Julian Rommel <julian@Julians-Mac-mini.local>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Improvements or additions to documentation community
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants