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

Code quality improvements to the Measurement Plug-In Client generator #901

Merged

Conversation

Jotheeswaran-Nandagopal
Copy link
Contributor

@Jotheeswaran-Nandagopal Jotheeswaran-Nandagopal commented Sep 19, 2024

What does this Pull Request accomplish?

  • Update docstring for pin_map_context in mako file.
  • Move helper method from __init__.py to _support.py in client generator.
  • Add protobuf enum to non_streaming_data_measurement for testing.
  • Remove os.linesep and tab spaces for output parameters, and handle the formatting in mako template using for loop.
  • Update the docstring for create_client method.

Why should this Pull Request be merged?

This Pull Request focuses on improving the overall quality and performance of the client generator code.

  • Refactoring of the code.
  • Adding an extra test case i.e., Protobuf enums.
  • Correcting link text in README files.
  • The create_client method's docstring is outdated. So, I have updated it based on the new change of moving the measurement-service-class to option from argument.

What testing has been done?

  • Existing test cases are passed.

Copy link

github-actions bot commented Sep 19, 2024

Test Results

    40 files  ±0      40 suites  ±0   48m 58s ⏱️ + 1m 24s
   689 tests ±0     689 ✅ ±0      0 💤 ±0  0 ❌ ±0 
16 790 runs  ±0  15 720 ✅ ±0  1 070 💤 ±0  0 ❌ ±0 

Results for commit 7247618. ± Comparison against base commit 13298f1.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jayaseelan-james jayaseelan-james left a comment

Choose a reason for hiding this comment

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

Regenerate the stubs for the non-streaming measurement.

Copy link
Collaborator

@bkeryan bkeryan left a comment

Choose a reason for hiding this comment

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

Approved with suggestions

@Jotheeswaran-Nandagopal Jotheeswaran-Nandagopal merged commit 82ab553 into main Sep 20, 2024
17 checks passed
@Jotheeswaran-Nandagopal Jotheeswaran-Nandagopal deleted the users/jothees/client-generator-improvements branch September 20, 2024 19:18
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.

5 participants