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

Update messaging fo the after merge comments #6178

Merged
merged 6 commits into from
Dec 6, 2022

Conversation

buyaa-n
Copy link
Contributor

@buyaa-n buyaa-n commented Sep 23, 2022

Improve messaging, addressing feedback in the PR

@@ -1990,7 +1990,7 @@
<value>Starting with .NET 7 the operator '{0}' will throw when overflowing in a checked context. Wrap the expression with an 'unchecked' statement to restore the .NET 6 behavior.</value>
</data>
<data name="PreventNumericIntPtrUIntPtrBehavioralChangesTitle" xml:space="preserve">
<value>Prevent from behavioral change</value>
<value>Prevent behavioral change</value>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Maybe better to mention that its related to the behavioral changes in IntPtr/UIntPtr, something like

Suggested change
<value>Prevent behavioral change</value>
<value>Prevent numeric IntPtr/UIntPtr behavioral changes</value>

@codecov
Copy link

codecov bot commented Sep 23, 2022

Codecov Report

Merging #6178 (f7fdd64) into main (585ec3f) will decrease coverage by 0.02%.
The diff coverage is n/a.

❗ Current head f7fdd64 differs from pull request most recent head 77ca277. Consider uploading reports for the commit 77ca277 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6178      +/-   ##
==========================================
- Coverage   96.06%   96.04%   -0.03%     
==========================================
  Files        1364     1359       -5     
  Lines      313730   311706    -2024     
  Branches    10125    10046      -79     
==========================================
- Hits       301379   299369    -2010     
+ Misses       9932     9924       -8     
+ Partials     2419     2413       -6     

Copy link
Member

@carlossanlop carlossanlop left a comment

Choose a reason for hiding this comment

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

I left some suggestions, they would need to get applied to the rest of the files.

@gewarren does my suggestion make sense, or would you word it differently?

@buyaa-n buyaa-n enabled auto-merge (squash) December 6, 2022 17:42
@buyaa-n buyaa-n merged commit 2928b85 into dotnet:main Dec 6, 2022
@github-actions github-actions bot added this to the vNext milestone Dec 6, 2022
@buyaa-n buyaa-n deleted the update-message branch December 6, 2022 18:46
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.

4 participants