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

Add ToSortedCollection() operator for ObservableCache and ObservableList #202

Merged
merged 1 commit into from
Feb 13, 2019

Conversation

modplug
Copy link
Contributor

@modplug modplug commented Feb 12, 2019

The good old ToCollection() does not maintain order. When ToCollection() is called it operates off the values, not the sorted values and thus I see the need for an operator that respect the sorted values

@@ -10,6 +10,7 @@

<ItemGroup>
<PackageReference Include="System.Reactive" Version="4.0.0" />
<PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" />
Copy link
Member

Choose a reason for hiding this comment

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

Wouldn't this be picked up due to the fact it's in the Directory.Build.Props file earlier, so this is redundant?

Copy link
Member

Choose a reason for hiding this comment

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

The other issue is you're not doing PrivateAssets="all" so it will cause nuget users of this framework to have to inherit this nuget package.

@@ -11,6 +11,7 @@
<ItemGroup>
<PackageReference Include="reactiveui" Version="9.0.1" />
<PackageReference Include="System.Reactive" Version="4.0.0" />
<PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" />
Copy link
Member

Choose a reason for hiding this comment

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

Same here, this should be picked up via Directory.Build.Props


<ItemGroup>
<PackageReference Include="System.Reactive" Version="4.0.0" />
<PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" />
Copy link
Member

Choose a reason for hiding this comment

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

In the ideal world you wouldn't have the sourcelink in the tests project anyway.

@@ -7,6 +7,7 @@

<ItemGroup>
<PackageReference Include="System.Reactive" Version="4.0.0" />
<PackageReference Update="Microsoft.SourceLink.GitHub" Version="1.0.0-beta2-18618-05" />
Copy link
Member

Choose a reason for hiding this comment

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

Same here re: directory.build.props

@modplug
Copy link
Contributor Author

modplug commented Feb 12, 2019

@glennawatson I reverted the SourceLink update commit

@modplug modplug force-pushed the master branch 3 times, most recently from 1fe9879 to 1ca74e7 Compare February 13, 2019 08:52
@RolandPheasant RolandPheasant merged commit 9bab938 into reactivemarbles:master Feb 13, 2019
@lock lock bot locked and limited conversation to collaborators Jul 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants