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

Update language in experimental RPC API paramaters #1155

Merged
merged 4 commits into from
Apr 5, 2024

Conversation

mattias-p
Copy link
Member

@mattias-p mattias-p commented Mar 6, 2024

Purpose

This PR updates the terminology used in experimental RPC API methods.

  • Use "job id" instead of "test id", "hash id" or just "id".
  • Avoid using to "frontend".
  • Abbreviate cardinal "number" as "num" instead of "nb". Is "nb" a french abbreviation? It feels foreign to me. (I also considered using "no" as in №, but that's for ordinal numbers and these are cardinal numbers.)

Context

Fixes #1084.

Changes

In the RPC API the following input and output parameters are renamed.

RPC method Parameter type Old parameter name New parameter name
job_status input test_id job_id
job_params input test_id job_id
job_results input id job_id
job_results output hash_id job_id
domain_history input frontend_params params
domain_history output history[].id history[].job_id
batch_create input test_params job_params
batch_status output finished_test_ids finished_job_ids
batch_status output nb_finished finished_count
batch_status output nb_running running_count

How to test this PR

Make calls to each RPC method in the table above and make sure it accepts the new input parameters and that the new output parameters are present in the output.

@mattias-p mattias-p marked this pull request as ready for review March 6, 2024 15:42
@mattias-p mattias-p added this to the v2024.1 milestone Mar 6, 2024
@marc-vanderwal
Copy link
Contributor

Yes, “nb” is a usual French shorthand for “nombre”. So variable names such as nb_running are technically what we call franglais. I agree that “nb” ought to be replaced with “num” or something to that effect.

Copy link
Contributor

@marc-vanderwal marc-vanderwal left a comment

Choose a reason for hiding this comment

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

This looks fine and I agree with the change; I only have some extremely minor suggestions.

lib/Zonemaster/Backend/RPCAPI.pm Outdated Show resolved Hide resolved
lib/Zonemaster/Backend/RPCAPI.pm Outdated Show resolved Hide resolved
mattias-p and others added 2 commits March 7, 2024 09:55
marc-vanderwal
marc-vanderwal previously approved these changes Mar 7, 2024
@hannaeko
Copy link
Member

May I suggest finished_count and running_count instead of num_finished and num_running, to get rid of the abbreviated num altogether?

@mattias-p
Copy link
Member Author

Sure, I like it. I'll make an update.

@mattias-p mattias-p merged commit a4bb361 into zonemaster:develop Apr 5, 2024
1 check passed
@mattias-p mattias-p added the V-Patch Versioning: The change gives an update of patch in version. label Jun 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
V-Patch Versioning: The change gives an update of patch in version.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants