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.

Sign up for GitHub

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

Jump to bottom

minimal utf-8 TextEncoder / TextDecoder API with example #3

Merged
merged 6 commits into from
Oct 14, 2020
Merged

minimal utf-8 TextEncoder / TextDecoder API with example #3

merged 6 commits into from
Oct 14, 2020

Conversation

dckc
Copy link
Collaborator

@dckc dckc commented Oct 13, 2020 •
edited
Loading

not sure which branch to target. public should probably track moddable's public

@dckc dckc changed the title minimal TextEncoder / TextDecoder API with example minimal utf-8 TextEncoder / TextDecoder API with example Oct 13, 2020
@dckc dckc force-pushed the text-utf8-enc-dec branch from d61745f to f2c806f Compare October 13, 2020 19:09
@dckc dckc changed the base branch from public to ag-linux-cli October 13, 2020 19:10
kriskowal
kriskowal approved these changes Oct 13, 2020
View reviewed changes
modules/data/text/text.js Outdated Show resolved Hide resolved
modules/data/text/text.js Outdated Show resolved Hide resolved
@dckc dckc force-pushed the text-utf8-enc-dec branch from e00c919 to be83ff7 Compare October 13, 2020 21:17
@dckc
Copy link
Collaborator Author

dckc commented Oct 13, 2020 •
edited
Loading

I'm still ambivalent about which branch.

  • This stuff doesn't depend on the ag-linux-cli special sauce, so maybe it should go in public with the intention of contributing it upstream.
  • It doesn't even need to be in the moddable SDK; it could be in the repo of apps built on top.
    • OTOH, this is a standard JS feature, so it probably belongs in the JS platform.

@warner ? @michaelfig ? Thoughts?

@michaelfig
Copy link

I'm still ambivalent about which branch.

  • This stuff doesn't depend on the ag-linux-cli special sauce, so maybe it should go in public with the intention of contributing it upstream.

  • It doesn't even need to be in the moddable SDK; it could be in the repo of apps built on top.

    • OTOH, this is a standard JS feature, so it probably belongs in the JS platform.

I think it's fine to put it in ag-linux-cli, with the intention of finding a better home for it later. No bits will be sacrificed to move it at that time (and maybe it will be made more complete).

michaelfig
michaelfig approved these changes Oct 14, 2020
View reviewed changes
Copy link

@michaelfig michaelfig left a comment

Choose a reason for hiding this comment

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

LGTM. Clear and to-the-point.

@dckc dckc merged commit 4c995ae into agoric-labs:ag-linux-cli Oct 14, 2020
@dckc dckc mentioned this pull request Nov 28, 2020
Closed
kriskowal pushed a commit that referenced this pull request Dec 28, 2021
XS: secaudit issue #3
c295400
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kriskowal kriskowal kriskowal approved these changes

@michaelfig michaelfig michaelfig approved these changes

@warner warner Awaiting requested review from warner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dckc @michaelfig @kriskowal