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

Some random clean up and improvements #11095

Merged
merged 4 commits into from
Oct 28, 2024

Conversation

DustinCampbell
Copy link
Member

These are just some tweaks I've had on my machine awhile.

  • Introduce PlatformInformation class to remove several RuntimeInformation.IsOSPlatform(...). These checks are fairly efficient, but each requires in a string comparison and it's easy enough to just do those once and remember the result.
  • Replace OSSkipConditionFactAttribute with ConditionalFactAttribute. This makes it easier to see what conditions a test will run under. For example, it's much easier to see that a test will run on Windows if it's marked with [ConditionalFact(Is.Windows)] rather than [OSSkipConditionFact(["Linux", "OSX"])]

This change move OSSkipConditionFactAttribute.cs in every test project and removes file links it in every test project.
This change replaces OSSkipConditionFactAttribute a more generally useful ConditionalFactAttribute (and ConditionalTheoryAttribute. This makes it easier to see what conditions a test *should* execute under rather that which conditions it shouldn't.
@DustinCampbell DustinCampbell requested review from a team as code owners October 25, 2024 23:10
@DustinCampbell
Copy link
Member Author

@dotnet/razor-compiler: I'm guessing this PR isn't too controversial, but it does make some small changes to compiler code. So, it'll need a couple of sign offs.

Copy link
Member

@jaredpar jaredpar left a comment

Choose a reason for hiding this comment

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

One question about the frozen dictionary but looks good otherwise.

@DustinCampbell DustinCampbell merged commit 76cd92a into dotnet:main Oct 28, 2024
12 checks passed
@DustinCampbell DustinCampbell deleted the random-clean-up branch October 28, 2024 22:34
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Oct 28, 2024
@phil-allen-msft phil-allen-msft modified the milestones: Next, 17.13 P1 Oct 31, 2024
@jjonescz jjonescz modified the milestones: 17.13 P1, Next Nov 5, 2024
phil-allen-msft added a commit that referenced this pull request Nov 6, 2024
This reverts commit 76cd92a, reversing
changes made to a37ee12.
phil-allen-msft added a commit that referenced this pull request Nov 6, 2024
Revert "Some random clean up and improvements (#11095) for dev17.13
@jjonescz jjonescz modified the milestones: Next, 17.13 P2 Nov 25, 2024
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.

7 participants