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

Add query_counter and get_query_parameter_u64_with_offset #260

Merged
merged 1 commit into from
Oct 16, 2023

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Oct 16, 2023

Not sure about the naming of get_query_parameter_ptr, as ptr doesn't correspond to u32 in get_query_parameter_u32.

@grovesNL
Copy link
Owner

Thanks! I'm not sure about the value: *mut () here - could we return the value directly instead?

@Zoxc
Copy link
Contributor Author

Zoxc commented Oct 16, 2023

It's an input, so that won't work. Maybe name it get_query_parameter_with_offset which takes usize?

@grovesNL
Copy link
Owner

Oh I see, yeah get_query_parameter_with_offset with usize sounds good!

@Zoxc Zoxc changed the title Add query_counter and get_query_parameter_ptr Add query_counter and get_query_parameter_with_offset Oct 16, 2023
@Zoxc
Copy link
Contributor Author

Zoxc commented Oct 16, 2023

Done.

@Zoxc Zoxc changed the title Add query_counter and get_query_parameter_with_offset Add query_counter and get_query_parameter_u64_with_offset Oct 16, 2023
@Zoxc
Copy link
Contributor Author

Zoxc commented Oct 16, 2023

I've changed this to use the 64-bit variant and renamed the function to get_query_parameter_u64_with_offset.

Copy link
Owner

@grovesNL grovesNL left a comment

Choose a reason for hiding this comment

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

Thank you!

@grovesNL grovesNL merged commit f4fa72e into grovesNL:main Oct 16, 2023
6 checks passed
@Zoxc Zoxc deleted the query_counter branch October 17, 2023 00:27
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.

2 participants