Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Code gen #40383

Closed
wants to merge 3 commits into from
Closed

Code gen #40383

wants to merge 3 commits into from

Conversation

pranavkm
Copy link
Contributor

@pranavkm pranavkm commented Feb 23, 2022

Follow up from the discussion in #40342

@pranavkm pranavkm marked this pull request as ready for review February 23, 2022 20:22
@pranavkm pranavkm requested review from a team, dougbu, wtgodbe and Pilchie as code owners February 23, 2022 20:22
Copy link
Member

@dougbu dougbu left a comment

Choose a reason for hiding this comment

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

This looks fine to me (as long as it works 😺).

Curious: How did you decide between the separate TrimmableProject.props option and adding %(TrimmableProject) metadata in ProjectReferences.props❔

Copy link
Member

@JamesNK JamesNK left a comment

Choose a reason for hiding this comment

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

Love it.

Out of curiosity, do you have an aspiration to eliminate the component's WASM linking test project? Setting the WASM build properties is simple enough. The more difficult part is distinguishing between trimmable WASM projects and trimmable aspnetcore projects.

eng/CodeGen.proj Outdated Show resolved Hide resolved
Co-authored-by: Doug Bunting <[email protected]>
@pranavkm
Copy link
Contributor Author

Curious: How did you decide between the separate TrimmableProject.props option and adding %(TrimmableProject) metadata in ProjectReferences.props❔

I followed the pattern for SharedFxDepList - assumed separate files were the way to go. Should I merge the contents?

Out of curiosity, do you have an aspiration to eliminate the component's WASM linking test project?

Right now, this test tests trimmability for all projects in the repo include the WebAssembly ones. I didn't do anything to exclude it since it appears to just work. If we can get some confirmation in the coming ways it doesn't get in the way of doing things, we can remove the WASM specific one.

@dougbu
Copy link
Member

dougbu commented Feb 23, 2022

Should I merge the contents?

Nope. Just wondered about reasonzzz; I have no objections to a separate file, especially because the added information is used in only a couple of places.

I'm doing something similar for #40242 and require a separate file because the info I'm adding isn't specific to project reference providers. That didn't apply here.

@JamesNK JamesNK deleted the branch dotnet:jamesnk/linkertest February 24, 2022 04:15
@JamesNK JamesNK closed this Feb 24, 2022
@JamesNK
Copy link
Member

JamesNK commented Feb 24, 2022

@pranavkm This got closed when I merged my PR. Rebase and open a new PR and I'll approve 🙏

pranavkm added a commit that referenced this pull request Feb 24, 2022
…cts to run LinkabilityChecker (#40394)

Re-doing #40383 since it closed.

This PR uses CodeGen to produce a list of projects in the repo that have <Trimmable>true</Trimmable>. This list is then used to produce the list of assemblies to test for trim correctness.
@amcasey amcasey added area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions and removed area-runtime labels Aug 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants