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

add a simple implementation of solarkraft list #64

Merged
merged 4 commits into from
May 17, 2024
Merged

add a simple implementation of solarkraft list #64

merged 4 commits into from
May 17, 2024

Conversation

konnov
Copy link
Contributor

@konnov konnov commented May 17, 2024

This PR implements solarkraft list. Since our file storage is very simple, the implementation is also simple.

@konnov konnov added the enhancement New feature or request label May 17, 2024
@konnov konnov added this to the M1: Transaction extractor milestone May 17, 2024
@konnov konnov self-assigned this May 17, 2024
Copy link
Collaborator

@Kukovec Kukovec left a comment

Choose a reason for hiding this comment

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

LGTM

path: string
): Generator<ListEntry> {
for (const dirent of readdirSync(path, { withFileTypes: true })) {
if (dirent.isDirectory() && /^[0-9]+$/.exec(dirent.name)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please add a comment explaining this regex

withFileTypes: true,
})) {
// match all storage entries
const matcher = /^entry-([0-9a-fA-F]+)\.json$/.exec(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why are we using mixed-case HEX here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I would be ok with lowercase, but neither Windows, nor MacOS agree with me on the capitalization of filenames

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah right, OS jank

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added a comment

@konnov konnov merged commit 5096a6b into main May 17, 2024
3 checks passed
@konnov konnov deleted the igor/list branch May 17, 2024 14:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants