Skip to content

Conversation

@kixelated
Copy link
Collaborator

No description provided.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 12, 2026

Caution

Review failed

The pull request is closed.

Walkthrough

Two clap argument configurations in the moq-native crate were changed to require equals-syntax parsing. The tls-disable-verify flag in rs/moq-native/src/client.rs and the iroh-enabled option in rs/moq-native/src/iroh.rs now include require_equals = true in their clap attribute annotations. This enforces providing boolean values with an equals sign (e.g., --tls-disable-verify=true).

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (2 inconclusive)
Check name Status Explanation Resolution
Title check ❓ Inconclusive The title 'require_equals' is vague and non-descriptive, using a generic term that doesn't convey meaningful information about the changeset without additional context. Provide a more descriptive title that clearly explains the change, such as 'Add require_equals constraint to CLI boolean flags' or 'Enforce equals syntax for tls-disable-verify and iroh-enabled flags'.
Description check ❓ Inconclusive No pull request description was provided by the author, making it impossible to assess whether it relates to the changeset. Add a description explaining the purpose and rationale for adding require_equals to these CLI arguments and any testing performed.
✅ Passed checks (1 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch test-iroh

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
rs/moq-native/src/client.rs (1)

355-358: ⚠️ Potential issue | 🔴 Critical

Test test_cli_disable_verify_explicit_false will fail with require_equals = true.

With require_equals = true, space-separated values like --tls-disable-verify false are no longer accepted — false will be treated as an unrecognized positional argument. The test must use the equals form.

🐛 Proposed fix
 	#[test]
 	fn test_cli_disable_verify_explicit_false() {
-		let config = ClientConfig::parse_from(["test", "--tls-disable-verify", "false"]);
+		let config = ClientConfig::parse_from(["test", "--tls-disable-verify=false"]);
 		assert_eq!(config.tls.disable_verify, Some(false));
 	}
🧹 Nitpick comments (1)
rs/moq-native/src/client.rs (1)

349-352: Consider also updating the test_cli_disable_verify_flag test for consistency.

While --tls-disable-verify (bare flag) still works due to num_args = 0..=1 + default_missing_value, adding the equals form here too would make the test suite consistently demonstrate the new required syntax.

@kixelated kixelated merged commit f90528e into main Feb 12, 2026
1 check passed
@kixelated kixelated deleted the test-iroh branch February 12, 2026 19:54
@moq-bot moq-bot bot mentioned this pull request Feb 12, 2026
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.

1 participant