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

Don't report CA2257 on nested types #7157

Merged
merged 1 commit into from
Feb 27, 2024

Conversation

jkoritzinsky
Copy link
Member

Fixes #7106

@jkoritzinsky jkoritzinsky requested a review from a team as a code owner January 23, 2024 19:59
Copy link

codecov bot commented Jan 23, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (3bdcce1) 96.33% compared to head (781d2ac) 96.44%.
Report is 468 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7157      +/-   ##
==========================================
+ Coverage   96.33%   96.44%   +0.11%     
==========================================
  Files        1389     1415      +26     
  Lines      325649   338212   +12563     
  Branches    10735    11191     +456     
==========================================
+ Hits       313699   326180   +12481     
+ Misses       9236     9212      -24     
- Partials     2714     2820     +106     

@carlossanlop
Copy link
Member

Since this is a servicing change, this would need to go through Tactics. Can you please fill out the typical template (like the one we use in runtime) and send an email to Tactics requesting approval?

@carlossanlop carlossanlop added the servicing-consider For backport requests to servicing branches. label Feb 15, 2024
@carlossanlop carlossanlop added this to the 8.0.2xx milestone Feb 15, 2024
@jkoritzinsky
Copy link
Member Author

This is not meant to be a servicing change. Which branch do I need to target for this to be non-servicing? Is it main or a different release/8.0.yxx branch?

@carlossanlop
Copy link
Member

It should go to main.

@sharwell sharwell changed the base branch from release/8.0.2xx to main February 15, 2024 15:10
@sharwell sharwell changed the base branch from main to release/8.0.2xx February 15, 2024 15:11
@carlossanlop carlossanlop removed the servicing-consider For backport requests to servicing branches. label Feb 15, 2024
@carlossanlop
Copy link
Member

carlossanlop commented Feb 15, 2024

I think it might be easier to resubmit the PR rather than retargeting the PR to the main branch. Otherwise a bunch of unrelated commits might end up getting included in the PR.

I suspect that's what @sharwell saw when attempting it and then reverting it in the two moves above.

@jkoritzinsky jkoritzinsky changed the base branch from release/8.0.2xx to main February 15, 2024 19:14
@jkoritzinsky
Copy link
Member Author

I rebased the PR onto main and then changed the branch, so it only has my commit.

@sharwell this is ready for review

@jkoritzinsky jkoritzinsky modified the milestones: 8.0.2xx, VS vNext Feb 20, 2024
Copy link
Member

@buyaa-n buyaa-n left a comment

Choose a reason for hiding this comment

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

LGTM

@jkoritzinsky jkoritzinsky merged commit 54ba3c9 into dotnet:main Feb 27, 2024
11 checks passed
@jkoritzinsky jkoritzinsky deleted the idic-fixes branch February 27, 2024 00:53
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.

CA2257 should not be reported on nested types
3 participants