Skip to content

Scriptorigin#918

Merged
kkoopa merged 4 commits intomasterfrom
scriptorigin
Aug 4, 2021
Merged

Scriptorigin#918
kkoopa merged 4 commits intomasterfrom
scriptorigin

Conversation

@kkoopa
Copy link
Copy Markdown
Collaborator

@kkoopa kkoopa commented Jul 10, 2021

To properly address #917 requires reimplementing v8::ScriptOrigin due to v8/v8@546939f (unintentionally?) breaking the old ScriptOrigin constructor by making resource_column_offset required. Since the same commit marked ScriptOrigin::ResourceLineOffset and ScriptOrigin::ResourceColumnOffset as soon to be deprecated, they should be reimplemented. Finally, @bnoordhuis decided to add a requirement for passing a v8::Isolate* in v8/v8@ee3f5ba, with the other constructors facing future deprecation.

@kkoopa
Copy link
Copy Markdown
Collaborator Author

kkoopa commented Jul 10, 2021

Seems no tests want to run. AppVeyor is acting weird for unknown reasons, while Travis claims that builds have been disabled due to lack of credits.

@kkoopa kkoopa force-pushed the scriptorigin branch 2 times, most recently from aa8f5bc to 99ae28a Compare August 2, 2021 17:51
@kkoopa
Copy link
Copy Markdown
Collaborator Author

kkoopa commented Aug 2, 2021

AppVeyor acting weird was due to VS2015 not being a proper compiler. Worked around it while sticking to C++03 by implementing forwarding constructors for all three supported constructors for all versions.

@kkoopa kkoopa force-pushed the scriptorigin branch 2 times, most recently from d263bd8 to 8b85f9b Compare August 3, 2021 06:55
@kkoopa kkoopa merged commit c0c0e01 into master Aug 4, 2021
@kkoopa kkoopa deleted the scriptorigin branch August 4, 2021 09:01
reconbot added a commit to serialport/node-serialport that referenced this pull request Sep 20, 2021
- debug (minor changes)
- [nan](nodejs/nan#918)
- prebuild-install (bug fixes)
reconbot added a commit to serialport/node-serialport that referenced this pull request Sep 20, 2021
- debug (minor changes)
- [nan](nodejs/nan#918)
- prebuild-install (bug fixes)
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