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.

Sign up for GitHub

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

Jump to bottom

[tests] Remove XASdkProject #7869

Merged
merged 1 commit into from
Jun 12, 2023
Merged

[tests] Remove XASdkProject #7869

merged 1 commit into from
Jun 12, 2023

Conversation

pjcollins
Copy link
Member

@pjcollins pjcollins commented Mar 9, 2023 •
edited
Loading

Replaces all instances of XASdkProject with XamarinProject types to
unify test project generation across all MSBuild suites.

@pjcollins pjcollins force-pushed the rmxasdktest branch 2 times, most recently from deaafd7 to 05b8b28 Compare March 10, 2023 18:30
@pjcollins pjcollins changed the title [tests] Stop using XASdkProject [tests] Remove XASdkProject Mar 31, 2023
@pjcollins pjcollins force-pushed the rmxasdktest branch 3 times, most recently from 6174250 to 7374fb5 Compare April 12, 2023 20:42
@pjcollins pjcollins force-pushed the rmxasdktest branch 2 times, most recently from d6217d1 to 3540137 Compare April 27, 2023 20:45
@pjcollins
Copy link
Member Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pjcollins pjcollins force-pushed the rmxasdktest branch 3 times, most recently from de10fd3 to 5cec7d6 Compare June 7, 2023 20:01
@pjcollins pjcollins marked this pull request as ready for review June 7, 2023 23:55
@pjcollins
[tests] Stop using XASdkProject
8928897 
Replaces all instances of `XASdkProject` with `XamarinProject` types to
unify test project generation across all MSBuild suites.
jonathanpeppers
jonathanpeppers approved these changes Jun 12, 2023
View reviewed changes
Copy link
Member

@jonathanpeppers jonathanpeppers left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Overall this is probably good to delete double-test code.

@pjcollins do you have a .csproj handy from what one of these projects generated?

I remember they seem to generate projects differently, related to <ItemGroup> -- so just wondering if something odd might happen there.

@pjcollins
Copy link
Member Author

@pjcollins do you have a .csproj handy from what one of these projects generated?

I remember they seem to generate projects differently, related to <ItemGroup> -- so just wondering if something odd might happen there.

I made some changes around this by setting EnableDefaultItems=true in a handful of the tests that used XASdkProject, and updated the project writer to also ignore EmbeddedResource items in that case.

Here's a couple of projects generated differently in this PR: https://gist.github.com/pjcollins/a8d1902c2a955379858ef8d2947eb56f

There is some extra noise in the Xamarin*Project types, but not too much. We could consider setting EnableDefaultItems=true as a default, remove some extra properties, and/or remove the OutputPath items in the future.

@jonathanpeppers
Copy link
Member

Ok the main difference is EnableDefaultItems=false, it's probably fine for now. 👍

@pjcollins pjcollins merged commit 0968abf into dotnet:main Jun 12, 2023
@pjcollins pjcollins deleted the rmxasdktest branch June 12, 2023 20:30
grendello added a commit to grendello/xamarin-android that referenced this pull request Jun 13, 2023
@grendello
Merge branch 'main' into blazor-hang
0de8a6f 
* main:
  [build] stop setting `$DOTNET_gcServer` (dotnet#8085)
  [tests] Remove XASdkProject (dotnet#7869)
  [Mono.Android] Fix NRT warnings in hand bound code (dotnet#8086)
  Bump to dotnet/installer@9e6f9a8b9e 8.0.100-preview.6.23310.1 (dotnet#8123)
  Localized file check-in by OneLocBuild Task (dotnet#8102)
  LEGO: Merge pull request 8122
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dellis1972 dellis1972 dellis1972 approved these changes

@jonathanpeppers jonathanpeppers jonathanpeppers approved these changes

@jonpryor jonpryor Awaiting requested review from jonpryor jonpryor is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@pjcollins @jonathanpeppers @dellis1972