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

SVM: Add doc comments, restrict visibility of some xfaces to crate #136

Merged
merged 1 commit into from
Mar 8, 2024

Conversation

dmakarov
Copy link

@dmakarov dmakarov commented Mar 7, 2024

Problem

Some functions in SVM crate are visible outside the crate, although they're not part of public API.

Summary of Changes

Restrict visibility of functions not used outside SVM crate to the crate only. Add documentation comments. Rearrange the methods of structs according to visibility moving public methods to the beginning of file.

@dmakarov dmakarov force-pushed the master branch 4 times, most recently from 0102efb to 3eae916 Compare March 7, 2024 22:48
@dmakarov dmakarov marked this pull request as ready for review March 7, 2024 23:01
@dmakarov dmakarov requested review from pgarg66 and LucasSte March 7, 2024 23:39
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 97.18310% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 81.8%. Comparing base (ba43f74) to head (3eae916).
Report is 3 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master     #136    +/-   ##
========================================
  Coverage    81.8%    81.8%            
========================================
  Files         838      838            
  Lines      226294   226465   +171     
========================================
+ Hits       185271   185416   +145     
- Misses      41023    41049    +26     

@dmakarov dmakarov force-pushed the master branch 2 times, most recently from f15d514 to 2888eee Compare March 8, 2024 17:00
LucasSte
LucasSte previously approved these changes Mar 8, 2024
svm/src/account_loader.rs Outdated Show resolved Hide resolved
svm/src/account_loader.rs Outdated Show resolved Hide resolved
@dmakarov dmakarov merged commit d88050c into anza-xyz:master Mar 8, 2024
35 checks passed
Copy link

@t-nelson t-nelson left a comment

Choose a reason for hiding this comment

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

in the future, please avoid making so many changes in the same pr. at least the test re-organization, docs comments and other refactoring were all valid, independent PRs

@dmakarov
Copy link
Author

dmakarov commented Mar 8, 2024

point taken

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.

4 participants