Skip to content
This repository was archived by the owner on Apr 4, 2024. It is now read-only.

Conversation

@hssahota2
Copy link
Collaborator

No description provided.

Copy link
Member

@nedtwigg nedtwigg left a comment

Choose a reason for hiding this comment

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

Great! Just a few small changes needed.

Comment on lines 20 to 23
class FS:
def fileRead(self, typedPath: "TypedPath") -> str:
# Placeholder return or raise NotImplementedError
raise NotImplementedError("fileRead is not implemented")
Copy link
Member

Choose a reason for hiding this comment

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

Selfie Kotlin's FS abstraction is for javascript within-browser testing. In Python, we can just read and write files, no FS needed.

Comment on lines 38 to 40
self.cache: Dict[TypedPath, WritableComment] = defaultdict(
lambda: WritableComment.NO_COMMENT
)
Copy link
Member

Choose a reason for hiding this comment

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

  • in reference implementation, this was an ArrayMap. It's fine to use dict instead, that's good and pythonic, but then TypedPath will need to implement __hash__
  • we don't want a default value, because we use null to determine if we need to do the calculation

@hssahota2 hssahota2 marked this pull request as ready for review March 6, 2024 01:06
@nedtwigg
Copy link
Member

nedtwigg commented Mar 6, 2024

Excellent!

@nedtwigg nedtwigg merged commit f8f51a4 into diffplug:main Mar 6, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants