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
Conversation
✅ Deploy Preview for infallible-bardeen-164bc9 ready!Built without sensitive environment variables
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.
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.
LGTM with a minor suggestion
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.
Approved. Added a few comments.
rsync | ||
) | ||
sudo dnf -y install "${packages[@]}" | ||
sudo alternatives --set python3 /usr/bin/python3.9 |
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.
Some systems may use Python 3.10 or 3.11. Can we remove the hard-coded 3.9?
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.
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 |
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.
Instead of piping output to /dev/null, maybe pass the -s / --silent option to curl?
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.
-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) |
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.
Do we really need support for building on Ubuntu 20.04?
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.
no point in removing it yet
docs/content/preview/contribute/core-database/build-from-src-almalinux.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-almalinux.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-almalinux.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-centos.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-centos.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-centos.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-ubuntu.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-ubuntu.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-ubuntu.md
Outdated
Show resolved
Hide resolved
docs/content/preview/contribute/core-database/build-from-src-almalinux.md
Outdated
Show resolved
Hide resolved
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.
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.