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.

Sign up for GitHub

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

Jump to bottom

chore: always specify the quoting metric explicitly #2566

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

chore: always specify the quoting metric explicitly #2566

wants to merge 2 commits into from

Conversation

RolandSherwin
Copy link
Member

No description provided.

@RolandSherwin RolandSherwin force-pushed the dont_return_empty_quotess branch from c33b061 to c0d1934 Compare December 23, 2024 11:22
@RolandSherwin RolandSherwin changed the title chore: always specify the metric explicitly chore: always specify the quoting metric explicitly Dec 23, 2024
@RolandSherwin RolandSherwin force-pushed the dont_return_empty_quotess branch from c0d1934 to 4d9ac69 Compare December 23, 2024 11:23
grumbach
grumbach requested changes Jan 2, 2025
View reviewed changes
Copy link
Member

@grumbach grumbach left a comment

Choose a reason for hiding this comment

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

Nice tweaks, but I'm not sure about the Option addition in the quoting metrics

ant-networking/src/cmd.rs Outdated Show resolved Hide resolved
ant-networking/src/lib.rs Outdated Show resolved Hide resolved
ant-networking/src/record_store_api.rs Outdated Show resolved Hide resolved
ant-node/src/node.rs Outdated Show resolved Hide resolved
@RolandSherwin RolandSherwin force-pushed the dont_return_empty_quotess branch from 4d9ac69 to 4166187 Compare January 9, 2025 14:48
grumbach
grumbach approved these changes Jan 9, 2025
View reviewed changes
Copy link
Member

@grumbach grumbach left a comment

Choose a reason for hiding this comment

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

LGTM, two questions about Result<Option<...>> return types

@RolandSherwin RolandSherwin force-pushed the dont_return_empty_quotess branch from 4166187 to 1d03d8a Compare January 17, 2025 11:42
@RolandSherwin RolandSherwin changed the base branch from main to rc-2024.12.1-hotfix5 January 17, 2025 11:43
@RolandSherwin RolandSherwin force-pushed the dont_return_empty_quotess branch from 1d03d8a to ee235bc Compare January 17, 2025 12:02
@RolandSherwin RolandSherwin changed the base branch from rc-2024.12.1-hotfix5 to main January 17, 2025 12:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grumbach grumbach grumbach approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@RolandSherwin @grumbach