Skip to content

Initial refactoring before fixing evaluation of methods on primitives. - #66704

Closed
ilonatommy wants to merge 1 commit into
dotnet:mainfrom
ilonatommy:methods-on-primitives-65707-cleanup
Closed

Initial refactoring before fixing evaluation of methods on primitives.#66704
ilonatommy wants to merge 1 commit into
dotnet:mainfrom
ilonatommy:methods-on-primitives-65707-cleanup

Conversation

@ilonatommy

Copy link
Copy Markdown
Member

As suggested in #66654 (review), it's a PR with an initial refactoring to make the methods-on-primitives PR look more legible.

@ilonatommy ilonatommy added arch-wasm WebAssembly architecture area-Debugger-mono labels Mar 16, 2022
@ilonatommy
ilonatommy requested a review from radical March 16, 2022 10:30
@ilonatommy
ilonatommy requested a review from marek-safar as a code owner March 16, 2022 10:30
@ilonatommy ilonatommy self-assigned this Mar 16, 2022
@ghost

ghost commented Mar 16, 2022

Copy link
Copy Markdown

Tagging subscribers to 'arch-wasm': @lewing
See info in area-owners.md if you want to be subscribed.

Issue Details

As suggested in #66654 (review), it's a PR with an initial refactoring to make the methods-on-primitives PR look more legible.

Author: ilonatommy
Assignees: ilonatommy
Labels:

arch-wasm, area-Debugger-mono

Milestone: -

@radical

radical commented Mar 18, 2022

Copy link
Copy Markdown
Member

What is the motivation for this change? FindVariableNMethodCall makes sense as a private nested class. It even has public fields, expected to be used only by the enclosing class.
Renaming the two classes is a good idea though.

I'm not against the idea of splitting, but it does come with the cost of losing the history on it. It would become difficult to track/blame for past changes. So, curious about the motivation.

@ilonatommy ilonatommy closed this Mar 18, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Apr 18, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

arch-wasm WebAssembly architecture area-Debugger-mono

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants