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

Moved to SDK RC2 #12254

Merged
merged 3 commits into from
Oct 3, 2024
Merged

Moved to SDK RC2 #12254

merged 3 commits into from
Oct 3, 2024

Conversation

Tanya-Solyanik
Copy link
Member

@Tanya-Solyanik Tanya-Solyanik commented Oct 2, 2024

Moved to SDK RC2 to get the same build errors in the VS and CLI build and be able to fix them.

Before this change the IntPreview version of VS was correctly complaining about a redundant cast(IDE0004) in ToolStrip.cs
g.DrawLines(SystemPens.ControlText, (ReadOnlySpan)
[
new(verticalBeamStart, _lastInsertionMarkRect.Y), new(verticalBeamStart, _lastInsertionMarkRect.Bottom - 1),
new(verticalBeamStart + 1, _lastInsertionMarkRect.Y), new(verticalBeamStart + 1, _lastInsertionMarkRect.Bottom - 1)
]);

But the CLI build required this cast.

After the upgrade to RC2, IDE0300 - Collection initialization can be simplified - became more robust and required code fixes that use collection expressions applied to the solution.

Microsoft Reviewers: Open in CodeFlow

… be able to fix them.

Before this change the IntPreview version of VS was correctly complaining about a redundant cast(IDE0004) in ToolStrip.cs
    g.DrawLines(SystemPens.ControlText, (ReadOnlySpan<Point>)
        [
            new(verticalBeamStart, _lastInsertionMarkRect.Y), new(verticalBeamStart, _lastInsertionMarkRect.Bottom - 1),
            new(verticalBeamStart + 1, _lastInsertionMarkRect.Y), new(verticalBeamStart + 1, _lastInsertionMarkRect.Bottom - 1)
        ]);

But the CLI build required this cast.

After the upgrade to RC2, IDE0300 - Collection initialization can be simplified - became more robust and required code fixes that use collection expressions applied to the solution.
Copy link

codecov bot commented Oct 2, 2024

Codecov Report

Attention: Patch coverage is 91.53439% with 16 lines in your changes missing coverage. Please review.

Project coverage is 75.43584%. Comparing base (c9a58e9) to head (363ffa4).
Report is 5 commits behind head on main.

Additional details and impacted files
@@                 Coverage Diff                 @@
##                main      #12254         +/-   ##
===================================================
+ Coverage   75.43315%   75.43584%   +0.00269%     
===================================================
  Files           3103        3103                 
  Lines         634314      634298         -16     
  Branches       46876       46875          -1     
===================================================
+ Hits          478483      478488          +5     
+ Misses        152407      152386         -21     
  Partials        3424        3424                 
Flag Coverage Δ
Debug 75.43584% <91.53439%> (+0.00269%) ⬆️
integration 17.98414% <0.00000%> (-0.00200%) ⬇️
production 48.82131% <11.11111%> (+0.00469%) ⬆️
test 97.02522% <100.00000%> (+0.00026%) ⬆️
unit 45.85356% <11.11111%> (+0.00632%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

_absoluteNumericUpDown.Maximum = new decimal(new int[]
{
_absoluteNumericUpDown.Maximum = new decimal(
[
Copy link
Member

Choose a reason for hiding this comment

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

We should move these away from the array constructor if we're touching them to avoid the extra allocation.

@@ -604,7 +604,7 @@ public void AddCurve_InvalidSegment_ThrowsArgumentException(int segment)
() => gp.AddCurve(new PointF[2] { new(1f, 1f), new(2f, 2f) }, 0, segment, 0.5f));

AssertExtensions.ThrowsAny<ArgumentException, ArgumentOutOfRangeException>(
() => gp.AddCurve(new Point[2] { new(1, 1), new(2, 2) }, 0, segment, 0.5f));
() => gp.AddCurve([new(1, 1), new(2, 2)], 0, segment, 0.5f));
Copy link
Member

Choose a reason for hiding this comment

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

It seems odd that we're only updating the Point overload (and not PointF).

Copy link
Member Author

@Tanya-Solyanik Tanya-Solyanik Oct 2, 2024

Choose a reason for hiding this comment

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

I only updated places that we detected by the rule. I think analyzer does not detect floats correctly, this is the difference between the versions as I see it.

Copy link
Member Author

@Tanya-Solyanik Tanya-Solyanik Oct 2, 2024

Choose a reason for hiding this comment

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

   AssertExtensions.ThrowsAny<ArgumentException, ArgumentOutOfRangeException>(
      () => gp.AddCurve([new(1f, 1f), new(2f, 2f)], 0, segment, 0.5f));

generates

Argument 1: cannot convert from 'float' to 'int'

in VS and CLI after the update to rc2

Copy link
Member Author

@Tanya-Solyanik Tanya-Solyanik Oct 2, 2024

Choose a reason for hiding this comment

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

Or maybe the difference is more specifically in the PointF rather than any float

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

Left a few comments. Also: please double check to make sure that the System.Drawing calls are binding to the span overloads and not the array ones.

@dotnet-policy-service dotnet-policy-service bot added 📭 waiting-author-feedback The team requires more information from the author and removed 📭 waiting-author-feedback The team requires more information from the author labels Oct 2, 2024
@Tanya-Solyanik
Copy link
Member Author

please double check to make sure that the System.Drawing calls are binding to the span overloads and not the array ones.

If both n array and ReadonlySpan overloads are available, we resolve the span one in VS editor and when running tests.

System.Drawing.Drawing2D.GraphicsPath.AddLines(System.ReadOnlySpan<System.Drawing.Point> points) Line 361 C#
System.Drawing.Drawing2D.Tests.GraphicsPathTests.AddLines_Success() Line 256 C#

0,
0
});
_percentNumericUpDown.Maximum = new decimal(lo: 9999, mid: 0, hi: 0, isNegative: false, scale: 0);
Copy link
Member

Choose a reason for hiding this comment

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

You should be able to just call new decimal(9999). The most performant one is new decimal(9999u), but not by much.

Copy link
Member

@JeremyKuhne JeremyKuhne left a comment

Choose a reason for hiding this comment

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

As long as everything is still going through the span overloads this is good.

@Tanya-Solyanik Tanya-Solyanik merged commit 5570dfa into dotnet:main Oct 3, 2024
8 checks passed
@Tanya-Solyanik Tanya-Solyanik deleted the move-to-rc2 branch October 3, 2024 01:23
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0 Preview1 milestone Oct 3, 2024
paul1956 pushed a commit to paul1956/winforms that referenced this pull request Oct 4, 2024
* Moved to SDK RC2 to get the same build errors in VS and CLI build and be able to fix them.

Before this change the IntPreview version of VS was correctly complaining about a redundant cast(IDE0004) in ToolStrip.cs
    g.DrawLines(SystemPens.ControlText, (ReadOnlySpan<Point>)
        [
            new(verticalBeamStart, _lastInsertionMarkRect.Y), new(verticalBeamStart, _lastInsertionMarkRect.Bottom - 1),
            new(verticalBeamStart + 1, _lastInsertionMarkRect.Y), new(verticalBeamStart + 1, _lastInsertionMarkRect.Bottom - 1)
        ]);

But the CLI build required this cast.

After the upgrade to RC2, IDE0300 - Collection initialization can be simplified - became more robust and required code fixes that use collection expressions applied to the solution.
lonitra added a commit that referenced this pull request Oct 11, 2024
…ifying PR #11863 (#12261)

* No code Chnages just formatting

* Add tests for SignleInstanceHelper from #11863

* Moved to SDK RC2 (#12254)

* Moved to SDK RC2 to get the same build errors in VS and CLI build and be able to fix them.

Before this change the IntPreview version of VS was correctly complaining about a redundant cast(IDE0004) in ToolStrip.cs
    g.DrawLines(SystemPens.ControlText, (ReadOnlySpan<Point>)
        [
            new(verticalBeamStart, _lastInsertionMarkRect.Y), new(verticalBeamStart, _lastInsertionMarkRect.Bottom - 1),
            new(verticalBeamStart + 1, _lastInsertionMarkRect.Y), new(verticalBeamStart + 1, _lastInsertionMarkRect.Bottom - 1)
        ]);

But the CLI build required this cast.

After the upgrade to RC2, IDE0300 - Collection initialization can be simplified - became more robust and required code fixes that use collection expressions applied to the solution.

* Adds XML Comments to FileSystemProxy and SpecialDirectoriesProxy (#12141)

* Add XML Comments related to FileSystemProxy which includes SpecialDirectoriesProxy

* Fix some types

* Add some language keywords

* Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/MyServices/FileSystemProxy.vb

Co-authored-by: Loni Tra <[email protected]>

* Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/MyServices/FileSystemProxy.vb

Co-authored-by: Loni Tra <[email protected]>

* Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/MyServices/FileSystemProxy.vb

Co-authored-by: Loni Tra <[email protected]>

* Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/MyServices/FileSystemProxy.vb

Co-authored-by: Loni Tra <[email protected]>

* PR feedback

* Update all XML comments

* Fix Typo in FileSystemProxy that caused build to fail.

* Update XML Coments

* Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/MyServices/SpecialDirectoriesProxy.vb

Co-authored-by: Tanya Solyanik <[email protected]>

* Update src/Microsoft.VisualBasic.Forms/src/Microsoft/VisualBasic/MyServices/SpecialDirectoriesProxy.vb

Co-authored-by: Tanya Solyanik <[email protected]>

* Redo all the comments in SpecialDirectoriesProxy

* Add cref per PR feedback and changed type to Namespace

* PR Feedback to add see cref's

---------

Co-authored-by: Loni Tra <[email protected]>
Co-authored-by: Tanya Solyanik <[email protected]>

* [main] Update dependencies from dotnet/runtime (#12266)

[main] Update dependencies from dotnet/runtime

* Fix IDE0002 in LogTests.cs (#12265)

Fix IDE0002 in LogTests.cs

* [main] Update dependencies from dotnet/arcade (#12271)

[main] Update dependencies from dotnet/arcade

* [main] Update dependencies from dotnet/runtime (#12272)

[main] Update dependencies from dotnet/runtime

* Improve code coverage for FileLogTraceListener from simplifying PR #11863  (#12264)

* Improve code coverage for FileLogTraceListener

* Update src/Microsoft.VisualBasic/tests/UnitTests/Microsoft/VisualBasic/Logging/FileLogTraceListenerTests.cs

Thanks I had no idea about boolData

Co-authored-by: Loni Tra <[email protected]>

---------

Co-authored-by: Loni Tra <[email protected]>

* Switch default feed to use wildcards only (#12268)

* Switch default feed to full wildcard

* Add other wildcards back

* Move local closer to usage

* PR Feedback

* Address PR Feedback

* PR Feedback

* PR Feedback inline private Sub, reuse commandLine

---------

Co-authored-by: Tanya Solyanik <[email protected]>
Co-authored-by: Loni Tra <[email protected]>
Co-authored-by: dotnet-maestro[bot] <42748379+dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Rich Lander <[email protected]>
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.

2 participants