Skip to content

Conversation

yu-iskw
Copy link
Contributor

@yu-iskw yu-iskw commented Jul 15, 2025

Resolves #402

@cla-bot cla-bot bot added the cla:yes label Jul 15, 2025
yu-iskw added 4 commits July 15, 2025 11:36
Signed-off-by: Yu Ishikawa <[email protected]>
Signed-off-by: Yu Ishikawa <[email protected]>
Signed-off-by: Yu Ishikawa <[email protected]>
@yu-iskw yu-iskw marked this pull request as ready for review July 16, 2025 00:07
@yu-iskw yu-iskw requested a review from a team as a code owner July 16, 2025 00:07
@yu-iskw
Copy link
Contributor Author

yu-iskw commented Jul 24, 2025

@joellabes I am sorry for bothering you. Can you please review the PR?

@joellabes
Copy link
Contributor

@dataders @dbeatty10 could one of you help move this and #407 forward please?

Comment on lines 634 to 635
// Disallow the '{key:value}' format for flow-style YAML syntax
// to prevent key:value: None interpretation: https://stackoverflow.com/a/70909331
Copy link
Contributor

Choose a reason for hiding this comment

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

This comment is misplaced here but could be removed if you move the stackoverflow link to the comment before vars.starts_with(...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved the comment before vars.tarts_with(....

serde_json = { workspace = true }
stringmetrics = { workspace = true }
strum = { workspace = true }
strum = { workspace = true, features = ["derive"] }
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this really necessary? If yes, can you move it to the root Cargo.toml?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed this. I needed this to pass the entire unit tests locally. However, it is't relevant to the pull request.

yu-iskw added 2 commits July 29, 2025 10:00
Signed-off-by: Yu Ishikawa <[email protected]>
Signed-off-by: Yu Ishikawa <[email protected]>
@yu-iskw yu-iskw requested a review from felipecrv July 29, 2025 01:06
@yu-iskw
Copy link
Contributor Author

yu-iskw commented Jul 29, 2025

@felipecrv Thank you for the feedback. I have updated the code. Can you take a look at them again?

@yu-iskw
Copy link
Contributor Author

yu-iskw commented Aug 14, 2025

@felipecrv Sorry for bothering you, please review the update?

@yu-iskw
Copy link
Contributor Author

yu-iskw commented Sep 17, 2025

@felipecrv @joellabes Any updates?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] --vars option can't handle YAML

3 participants