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

Moves FileList functions to their own file/class (Phases 3/4 from #1369) #1384

Closed
wants to merge 2 commits into from

Conversation

xsdg
Copy link
Contributor

@xsdg xsdg commented Jun 5, 2024

This also creates a protected FileData API, which includes methods that originally existed as static functions within filedata/filedata.cc. Those protected methods are now accessible to FileData and its child classes, without being made accessible to code outside of the FileData class.

This is part of the #1369 refactor (phases 3 and 4)

This also creates a protected FileData API, which includes methods that
originally existed as static functions within `filedata/filedata.cc`.  Those
protected methods are now accessible to FileData and its child classes, without
being made accessible to code outside of the FileData class.
@xsdg
Copy link
Contributor Author

xsdg commented Jun 5, 2024

@qarkai definitely going to want your thoughts on this

src/filedata/filelist.cc Outdated Show resolved Hide resolved
src/filedata.h Show resolved Hide resolved
That said, this renames `filelist_free` to `free_list` and `filelist_read*` to
`read_list*` in order to avoid confusion with the `free` and `read` function
names that are super common in C.
@xsdg xsdg requested a review from qarkai June 5, 2024 17:15
Copy link
Contributor

@qarkai qarkai left a comment

Choose a reason for hiding this comment

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

LGTM

@caclark
Copy link
Collaborator

caclark commented Jun 7, 2024

Implemented in commits d2eb115 a72d5ce

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.

3 participants