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: remove needless scripts #1949

Merged
merged 3 commits into from
May 14, 2024
Merged

docs: remove needless scripts #1949

merged 3 commits into from
May 14, 2024

Conversation

matklad
Copy link
Member

@matklad matklad commented May 13, 2024

  • there's no need for install.sh when it's just ./scripts/install_zig.sh && ./zig/zig build -Drelease -Dconfig=production
  • there's no need to call zig build install, install is the default step
  • git hook uses the wrong zig, and in general hooks are very developer-specific

README.md Outdated
@@ -179,7 +179,8 @@ First grab the sources and run the setup script:
```console
git clone https://github.com/tigerbeetle/tigerbeetle.git
cd tigerbeetle
scripts/install.sh
./scripts/install_zig.sh
Copy link
Contributor

Choose a reason for hiding this comment

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

Ah! Let's point the user to the quick start instructions above? They no longer need source to benchmark.

Copy link
Member Author

Choose a reason for hiding this comment

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

I removed this section altogether. I don't think it's possible to meaningfully use these benchmarks unless you know quite a bit more than we are saying here.

@@ -99,7 +99,7 @@ git clone ${var.git_url}
cd tigerbeetle
git checkout ${var.git_ref}
./scripts/install_zig.sh
./zig/zig build install -Drelease -Dconfig=production
./zig/zig build -Drelease -Dconfig=production
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this whole folder too, unless you think it's useful to point people to... It's not actively used.

Copy link
Member Author

Choose a reason for hiding this comment

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

👍

@cb22
Copy link
Contributor

cb22 commented May 13, 2024

(the macOS yaml still references install.sh)

* there's no need for `install.sh` when it's just
  `./scripts/install_zig.sh && ./zig/zig build -Drelease -Dconfig=production`
* there's no need to call `zig build install`, install is the default
  step
* git hook uses the wrong zig, and in general hooks are very
  developer-specific
I think it is quiet useless, because it doesn't explain what exactly is
benchmarked and how to interpret the results.
@matklad matklad added this pull request to the merge queue May 14, 2024
Merged via the queue into main with commit c50cdf5 May 14, 2024
25 checks passed
@matklad matklad deleted the matklad/no-scripts branch May 14, 2024 13:08
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.

None yet

2 participants