Skip to content

Conversation

yashLadha
Copy link
Contributor

Added more context around the TODO change required for achieving the
task. When destructuring the isolate and environment_vars from the
environment object, it is leading to recursive dependency and thus not
able to refactor it in a better way.

@nodejs-github-bot nodejs-github-bot added the c++ Issues and PRs that require attention from people who are familiar with C++. label Jan 30, 2021
@yashLadha
Copy link
Contributor Author

If needed I can work on refactoring it to reduce the recursion but effort vs impact seems odd.

@aduh95 aduh95 requested a review from joyeecheung January 30, 2021 09:43
@yashLadha yashLadha force-pushed the chore_minor_comment_addition branch 2 times, most recently from c05c74f to 78f142a Compare February 2, 2021 16:01
@yashLadha yashLadha requested a review from PoojaDurgad February 2, 2021 22:56
@nodejs-github-bot
Copy link
Collaborator

Add more context around the TODO change required for achieving the
task. When destructuring the isolate and environment_vars from the
environment object, it is leading to recursive dependency and thus not
able to refactor it in a better way.

PR-URL: nodejs#37140
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Pooja D P <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
@Trott Trott force-pushed the chore_minor_comment_addition branch from 78f142a to d6e9446 Compare February 6, 2021 22:37
@Trott Trott merged commit d6e9446 into nodejs:master Feb 6, 2021
@Trott
Copy link
Member

Trott commented Feb 6, 2021

Landed in d6e9446

danielleadams pushed a commit that referenced this pull request Feb 16, 2021
Add more context around the TODO change required for achieving the
task. When destructuring the isolate and environment_vars from the
environment object, it is leading to recursive dependency and thus not
able to refactor it in a better way.

PR-URL: #37140
Reviewed-By: Joyee Cheung <[email protected]>
Reviewed-By: Pooja D P <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
This was referenced Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c++ Issues and PRs that require attention from people who are familiar with C++.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants