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

Configure CODEOWNERS for changes to RPC code #5266

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

bthomee
Copy link
Collaborator

@bthomee bthomee commented Jan 30, 2025

High Level Overview of Change

To ensure changes to any RPC-related code are compatible with other services, such as Clio, the RPC team will be required to review them.

Context of Change

It has happened a few times that changes to RPC functions were made or new functionality introduced, for instance the errors returned upon failure, that did not work well with Clio.

Type of Change

  • Chore (no impact to binary, e.g. .gitignore, formatting, dropping support for older tooling)

@ximinez
Copy link
Collaborator

ximinez commented Jan 30, 2025

So, just to make things more complicated, it's not just the RPC directory. Several handlers call functions in other classes to get their current status. For example NetworkOPsImp::getServerInfo().

@bthomee
Copy link
Collaborator Author

bthomee commented Jan 30, 2025

So, just to make things more complicated, it's not just the RPC directory. Several handlers call functions in other classes to get their current status. For example NetworkOPsImp::getServerInfo().

I was expecting nothing less 🙂 I'll first get GitHub to be happy with the format of the file, and then add all the paths. I'll ask Clio's help as well in identifying these paths.

Copy link

codecov bot commented Jan 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 78.1%. Comparing base (1b75dc8) to head (35c0e9f).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5266     +/-   ##
=========================================
- Coverage     78.1%   78.1%   -0.0%     
=========================================
  Files          790     790             
  Lines        67340   67340             
  Branches      8155    8150      -5     
=========================================
- Hits         52585   52584      -1     
- Misses       14755   14756      +1     

see 1 file with indirect coverage changes

Impacted file tree graph

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