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

[docs] Add tldr section for coredb contributors #22172

Merged
merged 53 commits into from May 10, 2024

Conversation

jaki
Copy link
Contributor

@jaki jaki commented Apr 26, 2024

CoreDB contributor docs for compiling have separate pieces of
instructions with some beneficial commentary, but there is no fast way
to do it in one shot.

Add a tldr section for all the linux platforms that collects all the
separate instructions into one place. This means future edits to the
instructions may need to worry about keeping the two in sync, but the
convenience seems worth it. Tested using docker images centos:7,
ubuntu:20.04, ubuntu:22.04, almalinux/8-minimal. It would be nice to
create docker files using those images as a followup.

Copy link

netlify bot commented Apr 26, 2024

Deploy Preview for infallible-bardeen-164bc9 ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit 7cc5695
🔍 Latest deploy log https://app.netlify.com/sites/infallible-bardeen-164bc9/deploys/663adfd6861c160008d78ddb
😎 Deploy Preview https://deploy-preview-22172--infallible-bardeen-164bc9.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

The text following the large header "TLDR" gets rendered larger than
usual.  Try to add extra empty lines to perhaps avoid that.
It doesn't work.  Oh well.

This reverts commit 81ef903.
@jasonyb jasonyb added the area/documentation Documentation needed label Apr 26, 2024
@jasonyb jasonyb added this to In progress in Documentation via automation Apr 26, 2024
Copy link
Collaborator

@aishwarya24 aishwarya24 left a comment

Choose a reason for hiding this comment

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

LGTM with a minor suggestion

Copy link
Collaborator

@mbautin mbautin left a comment

Choose a reason for hiding this comment

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

Approved. Added a few comments.

rsync
)
sudo dnf -y install "${packages[@]}"
sudo alternatives --set python3 /usr/bin/python3.9
Copy link
Collaborator

Choose a reason for hiding this comment

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

Some systems may use Python 3.10 or 3.11. Can we remove the hard-coded 3.9?

Copy link
Contributor Author

@jaki jaki May 3, 2024

Choose a reason for hiding this comment

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

There is no python310 or python311 package yet on Almalinux 8 as far as I can tell. Also, this is based off of the python section below, so changing both copies of this could be considered out of scope of this PR.

sudo alternatives --set python3 /usr/bin/python3.9
latest_zip_url=$(curl -Ls "https://api.github.com/repos/ninja-build/ninja/releases/latest" \
| grep browser_download_url | grep ninja-linux.zip | cut -d \" -f 4)
curl -Ls "$latest_zip_url" | zcat | sudo tee /usr/local/bin/ninja >/dev/null
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of piping output to /dev/null, maybe pass the -s / --silent option to curl?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

-s is used. sudo tee is piping to /dev/null

git clone https://github.com/yugabyte/yugabyte-db
cd yugabyte-db
case "$VERSION_ID" in
20.04)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we really need support for building on Ubuntu 20.04?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no point in removing it yet

jaki added 2 commits May 7, 2024 19:12
According to Dwight:

> In page headings start at level 2, that's what the styles support - bump all the headings down a level, or I provided an alternative

Apply the review comment.
@jasonyb jasonyb merged commit f5a8ac0 into yugabyte:master May 10, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/documentation Documentation needed
Projects
Documentation
In progress
Development

Successfully merging this pull request may close these issues.

None yet

5 participants