Skip to content
This repository was archived by the owner on Mar 16, 2022. It is now read-only.

Metadata support for JavaScript#367

Merged
jroper merged 2 commits intocloudstateio:masterfrom
jroper:js-metadata-support
Jul 8, 2020
Merged

Metadata support for JavaScript#367
jroper merged 2 commits intocloudstateio:masterfrom
jroper:js-metadata-support

Conversation

@jroper
Copy link
Copy Markdown
Member

@jroper jroper commented Jun 24, 2020

Depends on #366 being merged first.

@jroper jroper force-pushed the js-metadata-support branch from 9db6965 to 15a7c19 Compare June 24, 2020 04:13
handleCommand(command) {
const ctx = this.createContext(command.id);
let metadata = new Metadata([]);
if (command.metadata && command.metadata.entries) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Perhaps superfluous, but would it not make sense to check that the length of the entries are > 0?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It will never be an empty array - it will be undefined if the array is empty, since when a sequence in protobuf is empty, nothing gets encoded, and then when protobufjs decodes that, the result is that it never sets the entries property.

}

repeated SideEffect side_effects = 4;
repeated SideEffect side_effects = 5;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

5?

@jroper jroper force-pushed the js-metadata-support branch from 15a7c19 to 9552207 Compare July 1, 2020 06:02
@jroper jroper force-pushed the js-metadata-support branch from 9552207 to 9fa75de Compare July 2, 2020 04:16
Copy link
Copy Markdown
Member

@pvlugter pvlugter left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread node-support/src/metadata.js Outdated
Co-authored-by: Peter Vlugter <pvlugter@users.noreply.github.com>
@jroper jroper merged commit 7c353c2 into cloudstateio:master Jul 8, 2020
@jroper jroper deleted the js-metadata-support branch July 8, 2020 06:14
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.

3 participants