[GitHub] [buildstream] abderrahim commented on pull request #1609: Directory API refactoring

[GitBox <gi...@apache.org>]

abderrahim commented on pull request #1609:
URL: https://github.com/apache/buildstream/pull/1609#issuecomment-1066163266


   I find it a little hard to review, it would be nice to split into smaller PRs for review. The following I think are uncontroversial and can easily be merged in a first batch:
   
   - storage: Rename VirtualDirectoryError -> DirectoryError
   - storage/directory.py: Remove _mark_changed() internal API.
   - storage/directory.py: Removed mark_unmodified/list_modified_paths
   - storage/directory.py: Make set_deterministic_user() an internal function
   - storage/directory.py: Added full mypy annotations to API surface
   - storage/directory.py: Added declaration for abstract internal method _get_underlying_directory()
   - storage: Raise only DirectoryError from Directory backends
   - storage/directory.py: Remove "report_written" from Directory.import_files()
   - storage/_casbaseddirectory.py: Stop importing FileBasedDirectory
   - storage/directory.py: Make get_size() an internal method
   
   
   I have a couple of comments on these:
   - storage/directory.py: Remove `can_link` parameter from Directory.import_files()
     Even though it is currently unused, there is potential to have a similar option that uses  the `move_files` hint  (by passing `--capture-allow-file-moves` to `buildbox-casd`). Can be done later though.
   - storage/directory.py: Make Directory.export_files() an internal function
     If we want to go for https://github.com/apache/buildstream/pull/1607#issuecomment-1059019265, we'll need to keep this public. Again, this can be made public later if needed (even if it's not at 2.0).
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@buildstream.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org