Skip to content

Conversation

@sjorsdonkers
Copy link
Contributor

Based on #709

Renames The Zig Scope struct to Context as it is closely modeling/wrapping the v8_context.

Renamed the Page's Context to main_context to clarify that it is holding the main Worlds context

@sjorsdonkers sjorsdonkers changed the title rename scope to context Rename scope to context May 27, 2025
@sjorsdonkers sjorsdonkers force-pushed the Take-scope-out-of-scope branch from 54e8432 to 49a39e2 Compare May 27, 2025 18:58
@sjorsdonkers sjorsdonkers force-pushed the Rename_scope_context branch from b4d64cb to 93bf396 Compare May 27, 2025 19:02
@sjorsdonkers sjorsdonkers force-pushed the Take-scope-out-of-scope branch from 49a39e2 to 7075120 Compare May 27, 2025 19:06
@sjorsdonkers sjorsdonkers force-pushed the Rename_scope_context branch from 93bf396 to ac4ae2e Compare May 27, 2025 19:07
@karlseguin
Copy link
Collaborator

I'm against this PR. I think naming things the same just makes communication more complicated.

I think we have a leak in the context.
Which context? Our context or the v8 context?

And just the ambiguity of a variable name context while reviewing a PR.

@sjorsdonkers
Copy link
Contributor Author

I'm very open an alternative name. I considered ExecutionContext.
The current name Scope I consider strictly incorrect and very confusing as Scope and Context are established concepts that should not be interchanged.
Previously this held the handle_scope and a context, but only the scope in the cases this was the main scope since there is only 1 scope per isolate, but there are 2 (possibly many) contexts. Since at maximum 1 of these can hold the scope it was removed from this struct in the previous PR. So at this stage there is no scopiness about this struct.
Additionally I do think it is a Context, because that is the main component it is wrapping and loading. So naming it very differently, introducing yet another new name is adding complication for things that are basically the same.

@karlseguin karlseguin requested a review from krichprollsch May 28, 2025 11:41
@sjorsdonkers sjorsdonkers marked this pull request as draft June 2, 2025 06:27
@sjorsdonkers sjorsdonkers deleted the branch Take-scope-out-of-scope June 6, 2025 08:37
@sjorsdonkers sjorsdonkers deleted the Rename_scope_context branch August 4, 2025 09:15
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.

3 participants