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: Try using traits for FileManager [DO NOT MERGE] #3845

Closed
wants to merge 30 commits into from

Conversation

kevaundray
Copy link
Contributor

Description

This removes the need for an actual file manager in the compiler. The compiler instead defines an interface/trait that abstracts this away.

The current impl just copies the methods from FileManager, I'd imagine that we'd want to make the methods less specific to a file system, but for now if this looks good, I think its a future todo, since we rely on being able have segments for what we are calling path.

Problem*

Resolves

Summary*

Additional Context

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [Exceptional Case] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@kevaundray
Copy link
Contributor Author

Merges in changes from #3846

@kevaundray
Copy link
Contributor Author

Made a PR to build off of this and it seems that to get a clean API, we need to at the least:

  • Not have FileId be an asscociated type for the trait: A lot of places use FileId and so we are forced to make all of those methods and structs generic just because they use FileId.

@kevaundray
Copy link
Contributor Author

Until a follow-up PR can be built which shows how we would use the FileManager trait in a clean way, I'd be okay with not merging this just yet

@kevaundray kevaundray changed the title chore: Try using traits for FileManager chore: Try using traits for FileManager [DO NOT MERGE] Dec 17, 2023
Base automatically changed from kw/make-file-manager-reference to master December 19, 2023 14:13
@TomAFrench
Copy link
Member

I'm going to close this as stale. Handling these merge conflicts will basically require a rewrite anyway.

@TomAFrench TomAFrench closed this Feb 12, 2024
@TomAFrench TomAFrench deleted the kw/file-manager-trait branch February 12, 2024 16:28
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