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

[WIP] Move linker to arcade #453

Closed
wants to merge 16 commits into from
Closed

[WIP] Move linker to arcade #453

wants to merge 16 commits into from

Conversation

sbomer
Copy link
Member

@sbomer sbomer commented Jan 31, 2019

I'm taking over where @cshung left off in the effort to build the linker using arcade. Thanks for the work @cshung!

I wanted to put out the WIP PR to get early feedback on the planned change. At this point most of what I've done is investigation. I've tried to summarize some of the open questions in #452.

@sbomer sbomer requested a review from marek-safar as a code owner January 31, 2019 00:40
@mrvoorhe
Copy link
Contributor

Indeed it does look like only the easiest tests were converted to xunit. How would it look to convert https://github.com/Unity-Technologies/linker/blob/master/linker/Tests/TestCases/TestSuites.cs to xunit?

I'II need to give this more thought.

@mrvoorhe
Copy link
Contributor

I was thinking about this more. Please break up and land the changes as independently as possible. Ideally the 2nd to last PR to land would be the csproj changes and the final PR would be the switch to xunit.

I might be able to hack together something that let's us pull the csproj change into Unity's fork, but once the xunit change lands Unity will stop staying in sync with upstream. It could take us weeks or months to make the changes in our infrastructure that would be needed to pull in the xunit change. The more changes that land before the xunit switch, the more changes I can pull in and reduce the chances of cherry-pick problems bringing our contributions to a halt.

For example, land any file renaming, or directory restructuring by themselves first.

In an ideal world, this effort would slow down and give us a chance to prepare so that when the csproj and xunit changes come, we are better prepared to adopt both.

@mrvoorhe
Copy link
Contributor

@sbomer While I won't/can't stop a switch to xunit, it's worth noting I have little interest in seeing such a change land. NUnit is in use for a reason. At Unity, all of IL2CPP's tests use nunit. UnityLinker is tightly coupled with our testing and build process for il2cpp, therefore UnityLinker also uses nunit.

NUnit wasn't just some arbitrary choice that was made. monolinker uses nunit because it makes life easier for the largest contributor of tests to send their work upstream.

@sbomer
Copy link
Member Author

sbomer commented Feb 5, 2019

@mrvoorhe, understood, thanks for the feedback! I'll focus on getting our build set up with arcade before trying to move tests to xunit. It's important to me that we retain the ability to collaborate with unity folks, and I'll do what I can to enable that.

@sbomer
Copy link
Member Author

sbomer commented Feb 16, 2019

I've made some progress getting the repo to build with the new project system, in a way that should also let the mono build continue to work. Now that I have a better idea of what that looks like, I'll be trying to make some independent changes that move in that direction, as you suggested. The first change is the directory restructuring: #466.

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.

4 participants