@mikejiang, I noticed some inconsistencies between our internal and external APIs with respect to the expected inputs for type in gh_pop_get_stats() and statistic in gs_pop_get_count_fast(). Internal APIs require leading capitalisation (e.g. "Count") whilst external APIs require lower case (e.g. "count").
To avoid any conflicts, I have converted the inputs for external APIs to lower case before calling match.arg(). This will ensure that any internal code using leading capitalisation for these arguments will still work with external APIs as well.
We should probably have a discussion to settle on a consistent convention for all APIs and then I can help to tidy up the code base to resolve any conflicts. In some cases, such as in gh_pop_get_stats_tfilter() leading capitalisation is required for the type argument for both internal and external APIs.
I have run all tests in test suite locally and they pass as expected.
mikejiang
commented
1 year ago
@mikejiang, I noticed some inconsistencies between our internal and external APIs with respect to the expected inputs for
typeingh_pop_get_stats()andstatisticings_pop_get_count_fast(). Internal APIs require leading capitalisation (e.g. "Count") whilst external APIs require lower case (e.g. "count").To avoid any conflicts, I have converted the inputs for external APIs to lower case before calling
match.arg(). This will ensure that any internal code using leading capitalisation for these arguments will still work with external APIs as well.We should probably have a discussion to settle on a consistent convention for all APIs and then I can help to tidy up the code base to resolve any conflicts. In some cases, such as in
gh_pop_get_stats_tfilter()leading capitalisation is required for thetypeargument for both internal and external APIs.I have run all tests in test suite locally and they pass as expected.