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

fix: Adjusting overrides to be single target and provide warnings #425

Merged
merged 3 commits into from
Nov 13, 2023
Merged

fix: Adjusting overrides to be single target and provide warnings #425

merged 3 commits into from
Nov 13, 2023

Conversation

nickrandolph
Copy link
Contributor

@nickrandolph nickrandolph commented Nov 10, 2023 •
edited
Loading

GitHub Issue (If applicable): closes #424 #423

PR Type

What kind of change does this PR introduce?

  • Bugfix

What is the current behavior?

If you add non-mobile overrides, the mobile project won't build.

What is the new behavior?

OverrideFrameworkVersions is now designed to be single target - if you uncomment more than one line in the solution-config.props it will pick up only the last

Validation messages have been added to build.props to help warn developers if they are trying to build a project with an invalid set of tfms/overrides

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tested code with current supported SDKs
  • Docs have been added/updated which fit documentation template. (for bug fixes / features)
  • Unit Tests and/or UI Tests for the changes have been added (for bug fixes / features) (if applicable)
  • Wasm UI Tests are not showing unexpected any differences. Validate PR Screenshots Compare Test Run results.
  • Contains NO breaking changes
  • Updated the Release Notes
  • Associated with an issue (GitHub or internal)

Other information

Internal Issue (If applicable):

@nickrandolph nickrandolph added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 10, 2023
@nickrandolph nickrandolph marked this pull request as draft November 10, 2023 13:13
@nickrandolph
Copy link
Contributor Author

Discussion with @dansiegel :

  • Going to see if we can extract DotNetVersion to avoid having overrides in the class libraries such as wasm, server, datacontract etc
  • Going to limit this overrides to only support specifying a single target framework for override - this will keep the logic simple and a clear focus for what this file is for (avoiding the VS overhead of muli-tfm projects)

@nickrandolph nickrandolph changed the title fix: Adjusting overrides to be incremental and provide warnings fix: Adjusting overrides to be single target and provide warnings Nov 13, 2023
@nickrandolph nickrandolph removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Nov 13, 2023
@nickrandolph nickrandolph marked this pull request as ready for review November 13, 2023 03:46
@nickrandolph nickrandolph force-pushed the dev/nr/fixoverrides branch 4 times, most recently from 8a0321e to 73f48bd Compare November 13, 2023 14:57
@nickrandolph nickrandolph force-pushed the dev/nr/fixoverrides branch 7 times, most recently from eef7d1a to c82e98a Compare November 13, 2023 16:06
@dansiegel dansiegel enabled auto-merge November 13, 2023 16:24
@dansiegel dansiegel merged commit 6b31f96 into main Nov 13, 2023
40 checks passed
@dansiegel dansiegel deleted the dev/nr/fixoverrides branch November 13, 2023 17:38
@dansiegel dansiegel mentioned this pull request Nov 13, 2023
Closed
jeromelaban
jeromelaban reviewed Nov 14, 2023
View reviewed changes
src/Uno.Templates/content/unoapp/Directory.Build.props
If working on a single target framework, copy solution-config.props.sample to solution-config.props
and uncomment the appropriate lines in solution-config.props to build for the desired platforms only.

https://platform.uno/docs/articles/guides/solution-building-single-targetframework.html
Copy link
Member

Choose a reason for hiding this comment

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

@dansiegel let's not use direct links in code, we can use this one instead: https://aka.platform.uno/uno-single-tfm-build

@nickrandolph
Copy link
Contributor Author

@Mergifyio backport release/stable/5.0

@mergify Mergify
Copy link

mergify bot commented Nov 17, 2023 •
edited
Loading

backport release/stable/5.0

✅ Backports have been created

@mergify mergify bot mentioned this pull request Nov 17, 2023
Merged
nickrandolph added a commit that referenced this pull request Nov 17, 2023
@nickrandolph
Merge pull request #454 from unoplatform/mergify/bp/release/stable/5.…
e9adca1 
…0/pr-425

fix: Adjusting overrides to be single target and provide warnings (backport #425)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jeromelaban jeromelaban jeromelaban left review comments

@MartinZikmund MartinZikmund MartinZikmund approved these changes

@dansiegel dansiegel dansiegel approved these changes

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

Successfully merging this pull request may close these issues.

Make solution-config.props.sample instructions clearer
4 participants
@nickrandolph @MartinZikmund @dansiegel @jeromelaban