Write JUMBF data into assets #45
Conversation
cyraxx
force-pushed
the
feature/jumbf-into-assets
branch
from
13ef113
to
7ad5766
Compare
cyraxx
force-pushed
the
feature/jumbf-into-assets
branch
from
e5cdc4f
to
bc19223
Compare
cyraxx
force-pushed
the
feature/jumbf-into-assets
branch
from
bc19223
to
dc9a6a0
Compare
cyraxx
changed the title
| /** | ||
| * Fills in the manifest store JUMBF into the previously created sapce. | ||
| */ | ||
| writeManifestJUMBF(jumbf: Uint8Array): Promise<void>; |
There was a problem hiding this comment.
Stupid question, why did you make these return promises? I would understand it if it was an async operation that possibly blocked on remote communication or took really long, but that isn't the case here. Also, those are the only two methods like that in this interface.
There was a problem hiding this comment.
I had the original idea that Asset implementations could also represent something that's not backed by a Uint8Array but by some other data repository that would require async operations – maybe reading from disk on demand instead of keeping the entire asset in memory, or some native implementation. No specific plans for that currently though.
| if (!asset) { | ||
| this.skip(); | ||
| } |
There was a problem hiding this comment.
Style question: Like this or inline
if (!asset) this.skip();I have myself not been consistent in this and wonder if there's a styleguide, like e.g. PHP's PSR-12.
There was a problem hiding this comment.
I'm not aware of a styleguide, but ESLint has a corresponding curly rule.
I've added 'curly': ['error', 'multi-or-nest', 'consistent'] to the ESLint config as a test, however that led to a lot of instances where we'd have to remove braces, and on first glance I'm not sure if I agree with all of them. Might be interesting to try this in a PR.
No description provided.