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

Land extension fixes #518

Merged
merged 10 commits into from
Aug 14, 2023
Merged

Conversation

bcomnes
Copy link
Contributor

@bcomnes bcomnes commented Aug 7, 2023

@jwerle Looks like these are needed in the latest extension work.

@jwerle jwerle requested review from jwerle, heapwolf and chicoxyzzy and removed request for jwerle and heapwolf August 8, 2023 17:18
api/dgram.js Show resolved Hide resolved
src/extension/extension.cc Outdated Show resolved Hide resolved
@@ -3609,7 +3609,7 @@ declare module "socket:dgram" {
*/
export class Socket extends EventEmitter {
constructor(options: any, callback: any);
id: bigint;
id: any;
Copy link
Member

Choose a reason for hiding this comment

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

hmm is this loose typing because of this.id = options?.id || rand64()?
can we type options bag?

@@ -176,7 +176,7 @@ constexpr auto gPListInfo = R"XML(<?xml version="1.0" encoding="UTF-8"?>
<key>DTSDKBuild</key>
<string>10.13</string>
<key>CFBundleVersion</key>
<string>v{{meta_version}}</string>
<string>{{meta_version}}</string>
Copy link
Member

Choose a reason for hiding this comment

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

oh good catch!

@chicoxyzzy
Copy link
Member

fs.promises.writeFile test failed on linux 😕

@chicoxyzzy
Copy link
Member

I think we can merge this

@bcomnes
Copy link
Contributor Author

bcomnes commented Aug 14, 2023

I think we can merge this

I'll double check with @jwerle today about it (its his commits).

@bcomnes bcomnes force-pushed the improve-extension-context-tree-and-builds branch from 6b5de3a to 792889a Compare August 14, 2023 16:34
@bcomnes
Copy link
Contributor Author

bcomnes commented Aug 14, 2023

Tests are failing for existing reasons. Landing this now.

@bcomnes bcomnes closed this Aug 14, 2023
@bcomnes bcomnes reopened this Aug 14, 2023
@bcomnes bcomnes merged commit 15335af into master Aug 14, 2023
4 of 8 checks passed
@bcomnes bcomnes deleted the improve-extension-context-tree-and-builds branch August 14, 2023 17:32
jwerle added a commit that referenced this pull request Aug 22, 2023
* refactor(src/extension): improve init state and context tree

* refactor(src/cli): allow building jsut an extension

* refactor(src): clean up / tidy

* refactor(include/socket/extension.h): include 'sapi_context_alloc' prototype

* refactor(api): clean up, listen 'connect'() for dgram

* fix(extension): remove from 'init' list after deinit

* refactor(src/extension/extension.cc: use 'std::make_shared'

* fix(src/cli/templates.hh): remove 'v' prefix in 'CFBundleVersion'

* chore(api/index.d.ts): generate typings

* Fix spacing

---------

Co-authored-by: Joseph Werle <[email protected]>
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.

4 participants