-
Notifications
You must be signed in to change notification settings - Fork 0
docs: update README and command descriptions, add certinfo screenshots #19
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
Conversation
📝 WalkthroughWalkthroughThe PR updates CLI command help descriptions to reference x.509 certificate terminology, removes demonstration video script files, and comprehensively rewrites the README to clarify the tool's dual functionality for HTTPS requests and certificate inspection with expanded installation guidance. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @README.md:
- Around line 159-161: Update the README sentence that currently reads "Binaries
and packages are built for Linux and MacOS, `amd64` and `arm64`." to use Apple's
official casing by replacing "MacOS" with "macOS" so it reads "Binaries and
packages are built for Linux and macOS, `amd64` and `arm64`."
- Around line 198-209: The Docker instructions use two different sample config
filenames (sample-wrench.yaml vs https-wrench-sample-config.yaml) and place the
--show-sample-config flag at the top-level; update the examples to use a single
filename (prefer https-wrench-sample-config.yaml everywhere), move
--show-sample-config to be invoked as part of the requests subcommand (e.g.,
docker run ... ghcr.io/xenos76/https-wrench:latest requests --show-sample-config
> https-wrench-sample-config.yaml), and adjust the run line to mount that same
https-wrench-sample-config.yaml and put the requests subcommand before --config
(e.g., docker run -v $(pwd)/https-wrench-sample-config.yaml:/https-wrench.yaml
--rm ghcr.io/xenos76/https-wrench:latest requests --config /https-wrench.yaml).
- Around line 220-224: The Homebrew install command in the README uses the
invalid flag `--casks`; update the install instruction for `https-wrench` to use
the correct syntax — either remove the flag (for a formula) or use the singular
`--cask` (for a cask). Replace the line containing `brew install --casks
https-wrench` with the appropriate `brew install https-wrench` or `brew install
--cask https-wrench` depending on whether `https-wrench` is a formula or cask.
🧹 Nitpick comments (1)
cmd/root.go (1)
52-52: Minor: Consider using "YAML" (uppercase) for consistency.The standard acronym is "YAML" (all uppercase). Line 54 uses "Yaml" while the README and other places mix "Yaml" and "YAML". Consider standardizing to "YAML" throughout.
Suggested change
- Short: "HTTPS Wrench, a tool to make Yaml defined HTTPS requests and inspect x.509 certificates and keys", + Short: "HTTPS Wrench, a tool to make YAML-defined HTTPS requests and inspect x.509 certificates and keys",
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (7)
assets/img/https-wrench-demo-k3s-example.gifis excluded by!**/*.gifassets/img/https-wrench-demo-sample-conf.gifis excluded by!**/*.gifassets/img/https-wrench-logo.jpgis excluded by!**/*.jpgassets/img/https-wrench_certinfo_cert_and_key.pngis excluded by!**/*.pngassets/img/https-wrench_certinfo_tls_endpoint.pngis excluded by!**/*.pngassets/img/https-wrench_requests_k3s.pngis excluded by!**/*.pngassets/img/https-wrench_requests_sample-config.pngis excluded by!**/*.png
📒 Files selected for processing (5)
README.mdassets/examples/vhs/https-wrench-demo-k3s-example.tapeassets/examples/vhs/https-wrench-demo-sample-conf.tapecmd/certinfo.gocmd/root.go
💤 Files with no reviewable changes (2)
- assets/examples/vhs/https-wrench-demo-sample-conf.tape
- assets/examples/vhs/https-wrench-demo-k3s-example.tape
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-11-08T16:00:48.187Z
Learnt from: xenOs76
Repo: xenOs76/https-wrench PR: 3
File: internal/requests/requests.go:164-182
Timestamp: 2025-11-08T16:00:48.187Z
Learning: In the https-wrench tool (xenOs76/https-wrench repository), do not enforce minimum TLS version constraints in the HTTP client configuration. The tool is designed to inspect and test TLS settings on various servers, including legacy ones, and needs the flexibility to connect to servers with older TLS versions.
Applied to files:
README.md
🪛 LanguageTool
README.md
[grammar] ~19-~19: Ensure spelling is correct
Context: ...is about to migrate a DNS record from a webserver to a load balancer, reverse proxy, Ingr...
(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
[uncategorized] ~161-~161: The operating system from Apple is written “macOS”.
Context: ...es and packages are built for Linux and MacOS, amd64 and arm64. ### APT Configu...
(MAC_OS)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: devenv_test
🔇 Additional comments (4)
cmd/root.go (1)
53-67: LGTM!The Long description clearly explains the two subcommands (
requestsandcertinfo) and their purposes. The terminology updates to reference "x.509 certificates" and "PEM encoded" formats are accurate and improve clarity.cmd/certinfo.go (1)
25-27: LGTM!The updated descriptions correctly use "x.509" terminology for certificate standards and clarify that the tool handles "PEM encoded" formats. This aligns well with the root command's updated descriptions.
README.md (2)
14-20: LGTM!The introduction clearly communicates the dual purpose of the tool (YAML-defined HTTPS requests and certificate inspection). The narrative about the tool's origins and aspirations adds personality to the documentation.
126-144: LGTM!The new screenshot sections for both
requestsandcertinfooutputs are well-organized using collapsible<details>elements, keeping the README clean while providing visual documentation for users.
| Release binaries and DEB, RPM, APK packages can be downloaded from the | ||
| [repo's releases section](https://github.com/xenOs76/https-wrench/releases).\ | ||
| Binaries and packages are built for Linux and MacOS, `amd64` and `arm64`. |
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.
Minor: Use "macOS" instead of "MacOS".
Per Apple's official branding, the operating system name is styled as "macOS" (lowercase "m", capital "OS").
Suggested fix
Release binaries and DEB, RPM, APK packages can be downloaded from the
[repo's releases section](https://github.com/xenOs76/https-wrench/releases).\
-Binaries and packages are built for Linux and MacOS, `amd64` and `arm64`.
+Binaries and packages are built for Linux and macOS, `amd64` and `arm64`.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Release binaries and DEB, RPM, APK packages can be downloaded from the | |
| [repo's releases section](https://github.com/xenOs76/https-wrench/releases).\ | |
| Binaries and packages are built for Linux and MacOS, `amd64` and `arm64`. | |
| Release binaries and DEB, RPM, APK packages can be downloaded from the | |
| [repo's releases section](https://github.com/xenOs76/https-wrench/releases).\ | |
| Binaries and packages are built for Linux and macOS, `amd64` and `arm64`. |
🧰 Tools
🪛 LanguageTool
[uncategorized] ~161-~161: The operating system from Apple is written “macOS”.
Context: ...es and packages are built for Linux and MacOS, amd64 and arm64. ### APT Configu...
(MAC_OS)
🤖 Prompt for AI Agents
In @README.md around lines 159 - 161, Update the README sentence that currently
reads "Binaries and packages are built for Linux and MacOS, `amd64` and
`arm64`." to use Apple's official casing by replacing "MacOS" with "macOS" so it
reads "Binaries and packages are built for Linux and macOS, `amd64` and
`arm64`."
| Generate the config: | ||
| ```bash | ||
| docker run --rm ghcr.io/xenos76/https-wrench:latest -h | ||
| docker run --rm ghcr.io/xenos76/https-wrench:latest --show-sample-config > sample-wrench.yaml | ||
| ``` | ||
| Run the `requests` command: | ||
| Run the `requests` command: | ||
| ```bash | ||
| docker run -v $(pwd)/sample-wrench.yaml:/https-wrench.yaml --rm ghcr.io/xenos76/https-wrench:latest --config /https-wrench.yaml requests |
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.
Inconsistent sample config filename in Docker instructions.
Line 65 uses https-wrench-sample-config.yaml as the new sample config filename, but line 203 still references sample-wrench.yaml. This inconsistency could confuse users following the documentation.
Suggested fix for consistency
docker run --rm ghcr.io/xenos76/https-wrench:latest -h
-docker run --rm ghcr.io/xenos76/https-wrench:latest --show-sample-config > sample-wrench.yaml
+docker run --rm ghcr.io/xenos76/https-wrench:latest requests --show-sample-config > https-wrench-sample-config.yamlRun the requests command:
-docker run -v $(pwd)/sample-wrench.yaml:/https-wrench.yaml --rm ghcr.io/xenos76/https-wrench:latest --config /https-wrench.yaml requests
+docker run -v $(pwd)/https-wrench-sample-config.yaml:/https-wrench.yaml --rm ghcr.io/xenos76/https-wrench:latest requests --config /https-wrench.yamlNote: The --show-sample-config flag likely belongs to the requests subcommand based on line 65, and the requests subcommand should come before --config in the run command.
🤖 Prompt for AI Agents
In @README.md around lines 198 - 209, The Docker instructions use two different
sample config filenames (sample-wrench.yaml vs https-wrench-sample-config.yaml)
and place the --show-sample-config flag at the top-level; update the examples to
use a single filename (prefer https-wrench-sample-config.yaml everywhere), move
--show-sample-config to be invoked as part of the requests subcommand (e.g.,
docker run ... ghcr.io/xenos76/https-wrench:latest requests --show-sample-config
> https-wrench-sample-config.yaml), and adjust the run line to mount that same
https-wrench-sample-config.yaml and put the requests subcommand before --config
(e.g., docker run -v $(pwd)/https-wrench-sample-config.yaml:/https-wrench.yaml
--rm ghcr.io/xenos76/https-wrench:latest requests --config /https-wrench.yaml).
| Install `https-wrench`: | ||
| ```bash | ||
| brew install --casks https-wrench | ||
| ``` |
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.
🧩 Analysis chain
🌐 Web query:
Homebrew brew install --cask vs --casks syntax documentation
💡 Result:
Short answer: use --cask (singular). The official brew manpage and Homebrew Cask docs show the flag as --cask (and the old brew cask subcommand was merged into brew); there is no documented --casks option. [1][2]
Sources:
- brew manpage (install / --cask). [1]
- Homebrew Cask / Cask Cookbook (installation examples use --cask). [2]
Incorrect Homebrew install syntax.
The --casks flag is not valid for brew install. Use --cask (singular) if installing a cask: brew install --cask https-wrench, or omit the flag entirely if it's a formula: brew install https-wrench.
Suggested fix (assuming it's a formula)
Install `https-wrench`:
```bash
-brew install --casks https-wrench
+brew install https-wrench🤖 Prompt for AI Agents
In @README.md around lines 220 - 224, The Homebrew install command in the README
uses the invalid flag `--casks`; update the install instruction for
`https-wrench` to use the correct syntax — either remove the flag (for a
formula) or use the singular `--cask` (for a cask). Replace the line containing
`brew install --casks https-wrench` with the appropriate `brew install
https-wrench` or `brew install --cask https-wrench` depending on whether
`https-wrench` is a formula or cask.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.