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

chore: Rename two files from Directory* to Dir*: #5058

Merged
merged 3 commits into from
Jul 25, 2024

Conversation

scottschurr
Copy link
Collaborator

High Level Overview of Change

The Dir class was contained in two files named Directory.h and Directory.cpp. The files are renamed Dir.h and Dir.cpp to reflect the class that they contain. Additionally:

  1. A few unnecessary includes of Directory.h have been removed and
  2. A little documentation of the Dir class has been added.

Type of Change

  • Refactor (non-breaking change that only restructures code)
  • Documentation update

The names of the files should reflect the name of the Dir class.
@scottschurr scottschurr added Tech Debt Non-urgent improvements Trivial Perf impact not expected Change is not expected to improve nor harm performance. labels Jul 9, 2024
@scottschurr scottschurr changed the title [chore] Rename two files from Directory* to Dir*: chore: Rename two files from Directory* to Dir*: Jul 9, 2024
Copy link

codecov bot commented Jul 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 71.4%. Comparing base (ad14d09) to head (9870f49).

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##           develop   #5058     +/-   ##
=========================================
- Coverage     71.4%   71.4%   -0.0%     
=========================================
  Files          796     796             
  Lines        67031   67031             
  Branches     10865   10866      +1     
=========================================
- Hits         47833   47828      -5     
- Misses       19198   19203      +5     
Files Coverage Δ
src/xrpld/app/paths/detail/BookStep.cpp 93.1% <ø> (ø)
src/xrpld/app/tx/detail/NFTokenBurn.cpp 98.1% <ø> (ø)
src/xrpld/app/tx/detail/NFTokenUtils.cpp 93.4% <ø> (ø)
src/xrpld/ledger/Dir.h 100.0% <ø> (ø)
src/xrpld/ledger/detail/Dir.cpp 87.0% <ø> (ø)

... and 4 files with indirect coverage changes

Impacted file tree graph

@zrayn zrayn requested a review from ximinez July 9, 2024 20:22
@scottschurr
Copy link
Collaborator Author

I requested @thejohnfreeman as a reviewer to make sure that I did not break some aspect of the build when I renamed the two files. John has directly confirmed to me that renaming those files should have no impact on the CMake stuff as it is currently structured. So I think that takes John off the hook as a reviewer. Thanks John!

@ximinez
Copy link
Collaborator

ximinez commented Jul 24, 2024

@scottschurr This PR now has two approvals. If you think it's ready to merge, please add the "Passed" label, or write a comment to that effect.

@scottschurr scottschurr added the Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. label Jul 24, 2024
@ximinez ximinez merged commit 20707fa into XRPLF:develop Jul 25, 2024
19 of 20 checks passed
vlntb pushed a commit to vlntb/rippled that referenced this pull request Aug 23, 2024
The names of the files should reflect the name of the Dir class.

Co-authored-by: Zack Brunson <[email protected]>
Co-authored-by: Ed Hennis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed Passed code review & PR owner thinks it's ready to merge. Perf sign-off may still be required. Perf impact not expected Change is not expected to improve nor harm performance. Tech Debt Non-urgent improvements Trivial
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants