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

fix :docs add docker-buildx installation step to requirements #288

Merged
merged 3 commits into from
Dec 18, 2024

Conversation

Priyanshuthapliyal2005
Copy link
Contributor

@Priyanshuthapliyal2005 Priyanshuthapliyal2005 commented Dec 14, 2024

Updated the Ubuntu setup instructions to include the installation of the docker-buildx plugin, which is necessary for building multi-platform Docker images.
Fixes #284
image

Signed-off-by: Priyanshu Thapliyal <[email protected]>
Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

Priyanshu, thanks very much for the contribution.

I have left a couple of suggestions in line.

Since we try to stick to conventional commits conventions, could you please also amend the commit message to read like:

fix(docs): add docker-buildx installation step to requirements

Thanks!

README.md Outdated

On Ubuntu you can do this with:

sudo apt install git docker.io jq tmux
sudo apt install docker-buildx
Copy link
Contributor

Choose a reason for hiding this comment

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

We could, more economically, add docker-buildx to L27 instead of using a separate apt install invocation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the suggestions, Thomas! I have:

Moved the Docker Buildx plugin installation step below the Docker-related setup section to cluster the information more tightly.
Combined the docker-buildx installation into the same apt install line for better readability.
Updated the commit message to reflect the correct conventional format.
Please let me know if there's anything else to improve!

Copy link
Contributor

Choose a reason for hiding this comment

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

Nice, thanks very much.

In the meantime, @Arnaud-de-Grandmaison-ARM has found an issue (and a fix) for a build issue that breaks the CI, see #290

When that PR is merged, you'll need to rebase your branch on top of main.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you for the update, @thomas-fossati! I’ll keep an eye on #290 and will rebase my branch on top of the main branch once the fix is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@thomas-fossati , I'm currently exploring some issues within the community to get more familiar with the organization. I've checked many repositories, and I've noticed that you are one of the most active mentors here. I would greatly appreciate it if you could suggest any specific issues or areas where I could contribute to improve and make a meaningful impact.
Thank you!

Copy link
Contributor

Choose a reason for hiding this comment

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

I would greatly appreciate it if you could suggest any specific issues or areas where I could contribute to improve and make a meaningful impact.

It depends on your skills and the amount of time you can commit.

I suggest joining https://veraison.zulipchat.com so we can have a one-on-one chat there.

README.md Outdated Show resolved Hide resolved
@Priyanshuthapliyal2005
Copy link
Contributor Author

Hi @thomas-fossati i have rebase my branch on top of main

Copy link
Contributor

@thomas-fossati thomas-fossati left a comment

Choose a reason for hiding this comment

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

Thanks @Priyanshuthapliyal2005 and welcome to Veraison :-)

@thomas-fossati thomas-fossati merged commit 95808b1 into veraison:main Dec 18, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

DOCS: missing installation instructions for the docker buildx plug-in
2 participants