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

port ops to JSON (compiler, errors, fetch, files) #2804

Merged
merged 4 commits into from
Aug 24, 2019
Merged

port ops to JSON (compiler, errors, fetch, files) #2804

merged 4 commits into from
Aug 24, 2019

Conversation

bartlomieju
Copy link
Member

@bartlomieju bartlomieju commented Aug 22, 2019 •
edited
Loading

Based on #2802

  • compiler.rs
  • errors.rs
  • fetch.rs
  • files.rs (sans, op_write and op_read)

@ry ry mentioned this pull request Aug 22, 2019
Closed
@bartlomieju
Copy link
Member Author

@ry how should we handle op_write and op_read that still use flatbuffers? Try to land again #2788 or do them in in JSON as hot fix and revisit later?

@ry
Copy link
Member

ry commented Aug 22, 2019

how should we handle op_write and op_read that still use flatbuffers?

Just leave them in flatbuffers for now. I'll port them over to dispatch_minimal later.

@bartlomieju bartlomieju changed the title WIP: port ops to json #3 port ops to JSON (compiler, errors, fetch, files) Aug 22, 2019
@bartlomieju
Copy link
Member Author

@ry this PR should be finished from my side. Please review, I'll rebase it once you land #2802

@bartlomieju bartlomieju force-pushed the port_ops_3 branch 2 times, most recently from b3bc54c to 720b169 Compare August 23, 2019 07:27
@bartlomieju
port ops to json ops
349c245 
* cli/ops/files.rs:
  * op_open
  * op_close
  * op_seek
* cli/ops/fetch.rs:
  * op_fetch
* cli/ops/compiler.rs:
  * op_cache
  * op_fetch_source_file
* cli/ops/errors.rs:
  * op_apply_source_map
  * op_format_error
@bartlomieju
Copy link
Member Author

@ry I rebased this PR on latest master, but it should be landed after #2802 because op ids are incremented from last id used in that PR

ry
ry reviewed Aug 23, 2019
View reviewed changes
js/dispatch.ts Show resolved Hide resolved
ry
ry reviewed Aug 23, 2019
View reviewed changes
js/fetch.ts Outdated Show resolved Hide resolved
ry
ry reviewed Aug 23, 2019
View reviewed changes
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

+327 −637

That's what I like to see : )

Looks good. Waiting for #2802.

@bartlomieju bartlomieju mentioned this pull request Aug 23, 2019
Merged
10 tasks
ry
ry approved these changes Aug 24, 2019
View reviewed changes
Copy link
Member

@ry ry left a comment

Choose a reason for hiding this comment

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

LGTM

@ry ry merged commit 79f82cf into denoland:master Aug 24, 2019
@bartlomieju bartlomieju deleted the port_ops_3 branch August 24, 2019 13:04
ry added a commit to ry/deno that referenced this pull request Aug 24, 2019
@ry
Revert "port ops to JSON: compiler, errors, fetch, files (denoland#2804…
cb6af04 
…)"

This reverts commit 79f82cf.
ry added a commit that referenced this pull request Aug 24, 2019
@ry
Revert json ops (#2814)
2235dd7 
* Revert "port more ops to JSON (#2809)"

This reverts commit 137f337.

* Revert "port ops to JSON: compiler, errors, fetch, files (#2804)"

This reverts commit 79f82cf.

* Revert "Port rest of os ops to JSON (#2802)"

This reverts commit 5b2baa5.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ry ry ry approved these changes

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

Successfully merging this pull request may close these issues.
2 participants
@bartlomieju @ry