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

Remove flatbuffers compile support #2795

Closed
wants to merge 2 commits into from
Closed

Remove flatbuffers compile support #2795

wants to merge 2 commits into from

Conversation

ry
Copy link
Member

@ry ry commented Aug 20, 2019

This commit checks into the repo the two generated files
cli/msg_generated.rs
js/msg_generated.js
And removes support for automatically building them from the build.

This is further simplifications in order to allow the cli crate to build
without gn/ninja. See #2608.

ry added 2 commits August 20, 2019 18:25
This is split into a seperate commit so it's easier to see the
non-generated diff. This commit doesn't actually make use of the new
files and likely the build is broken on this commit due to linting.
This commit checks into the repo the two generated files
  cli/msg_generated.rs
  js/msg_generated.js
And removes support for automatically building them from the build.

This is further simplifications in order to allow the cli crate to build
without gn/ninja. See denoland#2608.
@ry
Copy link
Member Author

ry commented Aug 20, 2019

Beware everyone - this PR marks the beginning of the end for flatbuffers (See #2121). It removes support for auto-generating msg_generated.ts and msg_generated.rs, so ease refactoring of the build system.

The two major hurdles of getting cli off gn/ninja is flatbuffers and snapshots. This takes care of the flatbuffer problem. The snapshot solution is more tricky and being worked on over at https://github.com/ry/deno_typescript ... I plan to merge deno_typescript work into this repo soon.

> git log --shortstat --pretty=oneline master...
733cb3a988a3607fee81733f0c837ad429102492 (HEAD -> remove_fbs, ry/remove_fbs) Remove flatbuffers compile support
 46 files changed, 67 insertions(+), 535 deletions(-)
ddaa997fa92bd4565d953ef72e54f3c81afbe421 Add generated flatbuffer code to repo
 3 files changed, 20992 insertions(+)

@ry ry requested a review from piscisaureus August 20, 2019 22:30
@piscisaureus
Copy link
Member

Any guidance on how to run flatbuffers manually should it be needed?
Or are JSON messages close enough for this to never be needed?

@ry
Copy link
Member Author

ry commented Aug 21, 2019

Porting ops to JSON is actually going smoothly. I want to delay ripping out the build support for flatbuffers, as I think we can do it better gradually as in #2799 ...

I hope to re-apply this in a few days but without having to add the generated files to repo.

@ry ry closed this Aug 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants