This repository has been archived by the owner on Sep 13, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 424
Add bulk operations to Collection<T> #1175
Merged
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,6 +18,7 @@ public Collection(System.Collections.Generic.IList<T> list) { } | |
bool System.Collections.IList.IsReadOnly { get { throw null; } } | ||
object System.Collections.IList.this[int index] { get { throw null; } set { } } | ||
public void Add(T item) { } | ||
public void AddRange(System.Collections.Generic.IEnumerable<T> collection) { } | ||
public void Clear() { } | ||
protected virtual void ClearItems() { } | ||
public bool Contains(T item) { throw null; } | ||
|
@@ -26,9 +27,15 @@ public void CopyTo(T[] array, int index) { } | |
public int IndexOf(T item) { throw null; } | ||
public void Insert(int index, T item) { } | ||
protected virtual void InsertItem(int index, T item) { } | ||
protected virtual void InsertItemsRange(int index, System.Collections.Generic.IEnumerable<T> collection) { } | ||
public void InsertRange(int index, System.Collections.Generic.IEnumerable<T> collection) { } | ||
public bool Remove(T item) { throw null; } | ||
public void RemoveAt(int index) { } | ||
protected virtual void RemoveItem(int index) { } | ||
protected virtual void RemoveItemsRange(int index, int count) { } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason why this and APIs bellow don't use Range type? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because this API was approved 3 years ago but only recently implemented. Not sure it's worth going back to changing the design. We can always add new overloads. |
||
public void RemoveRange(int index, int count) { } | ||
protected virtual void ReplaceItemsRange(int index, int count, System.Collections.Generic.IEnumerable<T> collection) { } | ||
public void ReplaceRange(int index, int count, System.Collections.Generic.IEnumerable<T> collection) { } | ||
protected virtual void SetItem(int index, T item) { } | ||
void System.Collections.ICollection.CopyTo(System.Array array, int index) { } | ||
System.Collections.IEnumerator System.Collections.IEnumerable.GetEnumerator() { throw null; } | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericstj @wtgodbe any idea where these are coming from? Those changes are new after I rebased to master. Did Arcade change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Anipik added better checks to catch the exact case that resulted in this bug.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this baseline file indicate BTW? netstandard 1.6 didn't have BaseNumberConverter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it did: https://github.com/dotnet/corefx/blob/82cce14f0c8188a2218ec5dc9a55977ccdba02e0/src/System.ComponentModel.TypeConverter/ref/project.json#L11
Or rather, we had a package that was applicable on netstandard1.6 and that package was absorbed into netstandard2.0.
This is telling you that you made BaseNumberConverter effectively sealed when compared to the previous version from the package. That's intentional, as you mentioned.
Closer i look this isn't actually a new check, probably this just missed being checked in with #1176 and didn't fail CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wtgodbe I don't understand exactly why this wouldn't have failed CI. Maybe you're auto-baselining. That's fine to do for local builds, but maybe you should turn it off on CI.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gotcha, so these changes are expected and we just didn't baseline those before.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ericstj good idea - this should be a quick fix, I'll whip something up now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might not have time today, I'll circle back tomorrow morning - #1184