Skip to content

Conversation

@nojaf
Copy link
Contributor

@nojaf nojaf commented Feb 23, 2023

I would like to add a (temporary or less temporary) test flag --test:DumpSignatureData that converts the mspec to json after ApplyExportRemappingToEntity.

This is to help in the investigation of #14734.

I was able to see that ReportWarnings can have a difference arg names when compiling multiple times using --graph mode.

image

Some samples.zip.

It is also interesting to compare graph versus non-graph.

If you are on board with having this test flag, visitEntity and visitVal probably could use improvements as I'm not quite sure what information I'm exactly after.

@nojaf nojaf requested a review from a team as a code owner February 23, 2023 09:14
@vzarytovskii vzarytovskii enabled auto-merge (squash) February 23, 2023 11:36
Copy link
Contributor

@psfinaki psfinaki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the wording "less temporary" :)

One suggestion is to split the new code in TypedTreeOps somewhere. It breaks my heart to see the file that big and getting even bigger. But if you don't have an idea about how to easily do that - that's okay.

Edit: oh it got auto-merged 🤷

@vzarytovskii vzarytovskii merged commit d56f9d7 into dotnet:main Feb 23, 2023
@vzarytovskii
Copy link
Member

Splitting big files should just be a separate churn

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

3 participants