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

Subsystems that require backporting attention for 8.x #354

Closed
MylesBorins opened this issue Aug 8, 2018 · 13 comments
Closed

Subsystems that require backporting attention for 8.x #354

MylesBorins opened this issue Aug 8, 2018 · 13 comments

Comments

@MylesBorins
Copy link
Contributor

@nodejs/timers
@nodejs/http2
@nodejs/n-api
@nodejs/v8-inspector

assert also has a deviance, but I don't know if it makes sense to do a backporting effort for it

@ofrobots
Copy link

ofrobots commented Aug 8, 2018

I believe @kjin is working on http2 back ports right now.

@mhdawson
Copy link
Member

mhdawson commented Aug 8, 2018

@MylesBorins I think we had done a pass at backporting the commits we thought were important for the next SemVer. Is it just that not everything has been backported for n-api or are there some specific concerns? @gabrielschulhof

@gabrielschulhof
Copy link

nodejs/node#21733 and nodejs/node#21732 have landed. I don't know of any others that have not landed cleanly.

@gabrielschulhof
Copy link

gabrielschulhof commented Aug 8, 2018

Though I guess we may also want 978d89f because that prevents memory leaks with function factories.

@gabrielschulhof
Copy link

@mhdawson do we want to backport semver-minor stuff like bigint?

@MylesBorins
Copy link
Contributor Author

MylesBorins commented Aug 8, 2018 via email

@mhdawson
Copy link
Member

mhdawson commented Aug 8, 2018

@agreed with 978d89f.

EDITED to be specific to N-API
My first thought on N-API semver-minor stuff is that unless it is causing problems for backporting we may want to wait until features exit experimental before backporting. Not opposed to us doing so, just not in a rush to do it unless we think there is a pull. My hope is that because most of the changes are localized to a few files it won't cause problems. We can then pull a batch of stuff back when it makes sense.

@mcollina
Copy link
Member

mcollina commented Aug 9, 2018

@mhdawson I have several modules that I've used in my HTTP2 experiments that cannot run on Node 8.x.

@mhdawson
Copy link
Member

mhdawson commented Aug 9, 2018

@mcollina, sorry I should have been more specific, my comments where only for N-API sermver minor changes.

@MylesBorins
Copy link
Contributor Author

last call for updates. We only have one more release until maintenance

@MylesBorins
Copy link
Contributor Author

oops... meant to ping.

It appears that @nodejs/n-api has drifted again. Below are commits that are on 10.x but not 8.x

  • 9b951601e2 - (HEAD -> v10.x-staging) n-api: make per-Context-ness of napi_env explicit (2 minut
    es ago)
  • 76453f1878 - src: replace deprecated uses of FunctionTemplate::GetFunction (9 weeks ago)
  • 5d5c3fab25 - src: refactor Environment::GetCurrent() usage (10 weeks ago)
  • bb2bbc8ebe - n-api: add generic finalizer callback (10 weeks ago)
  • 0c3242862a - src: make FIXED_ONE_BYTE_STRING an inline fn (3 months ago)
  • cf1a020917 - n-api: clean up thread-safe function (3 months ago)
  • 12f5480d55 - src: use String::Utf8Length with isolate (3 months ago)<Michaël Zasso>
  • f576425841 - src: use String::Write{OneByte,Utf8} with isolate (3 months ago)<Michaël Zasso>
  • b30b5de85d - src,deps: add isolate parameter to String::Concat (3 months ago)<Michaël Zasso>
  • 8eae030d1c - n-api: remove idle_running from TsFn (3 months ago)
  • e0336b2891 - src: fix may be uninitialized warning in n-api (4 months ago)
  • 45816c50ac - n-api: guard against cond null dereference (4 months ago)
  • 3096ee5a4b - napi: add bigint support (5 months ago)
  • c87037286f - n-api: fix compiler warning (5 months ago)
  • 91384bfe5f - n-api: add API for asynchronous functions (5 months ago)
  • 169bff3e9e - n-api: name CallbackBundle function fields (6 months ago)
  • 9047c8182c - n-api: remove unused napi_env member (6 months ago)
  • 53f8563353 - n-api: back up env before async work finalize (6 months ago)
  • 57dfd64f8f - src: add missing override to ThreadPoolWork funcs (7 months ago)
  • 2347ce8870 - src: unify thread pool work (7 months ago)
  • 8995408748 - src: keep track of open requests (7 months ago)
  • cd83df386b - n-api: initialize a module via a special symbol (7 months ago)
  • 80c46c1576 - src: remove MarkIndependent() calls (7 months ago)
  • c6c957d3cc - src: fix upcoming V8 deprecation warnings (8 months ago)
  • d4024815b7 - src: remove unnecessary Reset() calls (9 months ago)
  • 992703f2b5 - src: prevent persistent handle resource leaks (9 months ago)

We also appear to have a behavior difference in 8.x vs 10.x for @nodejs/http2

@mhdawson
Copy link
Member

The most important one I see would be:

53f8563353 - n-api: back up env before async work finalize (6 months ago)

@gabrielschulhof I'm not sure if you will have a chance to review as well? A number of them are related to the thread safe functionality.

This is probably the last chance to get the thread safe functionality into 8.x.

It might also be nice to get bb2bbc8ebe - n-api: add generic finalizer callback

but would like your view on how important it is to get those 2 in.

@MylesBorins
Copy link
Contributor Author

8.x is now in maintenance mode, closing

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

No branches or pull requests

5 participants