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

Fixup typings vertx 3.3.2 #11

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

DanielJoyce
Copy link

This fixes building typescript d.ts files for vertx 3.3.2.

  • Work around for jdk bug that was blocking template generation
  • Cleanup of template
  • Removing formatting step of final typings copy

@michel-kraemer
Copy link
Owner

Thank you so much for all your work. I'm just catching up with it...

This pull request seems to be very good. I haven't tested it yet. That's the next thing I'll do.

In the meantime, could you please help me cleanup the two github repositories a bit. You've created a lot of issues and pull requests and all of them are still open. I don't know which one to process first. I suppose most of them are superseded by this PR here, right? If so, please put a link to this PR in each issue and then close it. Thanks a lot!

Just a quick question regarding your code. Why was it important that you put the global definitions in a separate source file globals instead of keeping them in java and console?

Cheers,
Michel

This was referenced Aug 4, 2016
@DanielJoyce
Copy link
Author

Java and console are both tiny, and ambient type definitions have no impact on code size.

Since all of these are defined as globals in the verticle that loads TS files and are thus always available, it made sense to me that they also be in one single file.

It also simplified the template, removing the if-else test for generating special global declarations (vertx/console) only when it was processing the main Vertx class.

TL;DR; it simplifies the codegen template and provides a single point of reference for the globals injected by the verticle. If the verticle ever adds another global, then we know which file to put the ambient type in.

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