Skip to content

Conversation

@martijn00
Copy link
Contributor

What kind of change does this PR introduce? (Bug fix, feature, docs update, ...)

Feature

What is the current behavior? (You can also link to an open issue here)

Say you want to target MonoAndroid81 instead of the current MonoAndroid80, you need to change all properties in the csproj file.

What is the new behavior (if this is a feature change)?

It is now easier because the names should only contain the platform.

What might this PR break?

Nothing

Please check if the PR fulfills these requirements

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

Other information:

@ghuntley
Copy link
Member

ghuntley commented May 9, 2018

Aww shucks @martijn00. I was totally going to steal this from mvx and you have done it for us. Thank-you. ❤️

@martijn00
Copy link
Contributor Author

You could even change the folder names just like Mvx to be without the version number. Say you switch from netstandard2.0 to 2.1, the code probably still works. It would also be possible to target 1.0 and 2.0 from 1 folder. I just didn't want to make this change too big.

@ghuntley
Copy link
Member

ghuntley commented May 9, 2018

Good call - yes changing folder names would be too big of a change right now.

EDIT: I've changed my mind - let's change the folder names.

@ghuntley
Copy link
Member

@olevett I've broken something - the API approval tests are tripping. Any chance you could lend me a fresh set of eyes?

@ghuntley ghuntley requested a review from olevett May 14, 2018 05:47
@ghuntley ghuntley changed the title Make it easier to target new versions [WIP]: Make it easier to target new versions May 14, 2018
@ghuntley
Copy link
Member

ghuntley commented May 14, 2018

#1640 needs to be merged before this.

edit:

It's been merged.

@ghuntley ghuntley changed the base branch from develop to master May 14, 2018 20:00
@ghuntley ghuntley requested review from a team May 14, 2018 20:00
@olevett
Copy link
Member

olevett commented May 15, 2018

Will take a look...

@ghuntley
Copy link
Member

ghuntley commented May 16, 2018

👍 thanks Olly. This will need some additional eyeballs/changes as that big winforms PR as part of v8.2.1 was merged and this PR doesn't account for it atm.

<PublishRepositoryUrl>true</PublishRepositoryUrl>
<RepositoryUrl>https://github.com/reactiveui/ReactiveUI</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<Product>$(AssemblyName) ($(TargetFramework))</Product>
Copy link
Member

Choose a reason for hiding this comment

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

Just a thought, is this behaving here? There's another PR around it I think where AssemblyName is sometimes null?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let the other review do this, since it's fixing it properly.

@glennawatson
Copy link
Contributor

Given we have some conflicts can we get those fixed then we can work on getting this one integrated across?

<PublishRepositoryUrl>true</PublishRepositoryUrl>
<RepositoryUrl>https://github.com/reactiveui/ReactiveUI</RepositoryUrl>
<RepositoryType>git</RepositoryType>
<Product>$(AssemblyName) ($(TargetFramework))</Product>
Copy link
Contributor

Choose a reason for hiding this comment

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

Let the other review do this, since it's fixing it properly.

@glennawatson
Copy link
Contributor

The other review in question is #1604

@martijn00 martijn00 requested review from a team August 17, 2018 21:32
@glennawatson glennawatson changed the title [WIP]: Make it easier to target new versions housekeeping: Use StartsWith in the csproj files to make it easier to target new versions Aug 17, 2018
@glennawatson glennawatson merged commit 56a47a8 into reactiveui:master Aug 17, 2018
@glennawatson
Copy link
Contributor

Thanks @martijn00 - Our PR list is now empty, thanks for the contribution 👍

glennawatson pushed a commit that referenced this pull request Mar 23, 2019
@lock lock bot locked and limited conversation to collaborators Jun 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants