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

Adds various new conventions #89

Merged

Conversation

flakey-bit
Copy link
Contributor

This PR adds various new conventions (discussed in more detail in #87)

Things to note:

  • I've not added NamespaceMustStartWith because it turns out something equivalent already existed (not sure how I missed it)
  • I've opened an issue Assembly (project) conventions should support Directory.Build.Props & Directory.Build.Targets #88 which affects at least one existing convention but also affects most of the conventions this PR introduces
  • To support legacy csproj files, the code uses an XPath that explicitly looks only at the local name of elements, however let me know if you'd prefer a different approach.

@flakey-bit flakey-bit force-pushed the feature/GH-87-new-conventions branch from 62e51a9 to 34559b1 Compare January 6, 2024 03:04
@@ -515,5 +515,63 @@ public void LibraryCodeShouldCallConfigureAwaitWhenAwaitingTasks_FailsWhenConfig
result.IsSatisfied.Should().BeFalse();
result.Failures.Single().Should().Be(expectedFailureMessage);
}

# region MustNotUseGuidNewGuid
Copy link
Contributor Author

Choose a reason for hiding this comment

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

remark: I've introduced a region here to make it obvious that all of this is associated with the MustNotUseGuidNewGuid convention.

Might be worth splitting this file up.

Copy link
Owner

Choose a reason for hiding this comment

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

I'm not too worried about splitting these test files up, they tend to be easy enough to peruse. Can we remove the #region though? It conflicts with my core beliefs 😆

eddie.stanley added 6 commits January 5, 2024 19:20
…ode doesn't directly call Guid.NewGuid()

A possible use-case is code written in a functional style (that wants to avoid non-deterministic behaviour)
…included (by name) in a haystack set of assemblies

This can be used to ensure a list of assemblies (e.g. those containing "production code") is kept up-to-date *so that you can apply conventions to the types in those assemblies*.
…ct doesn't include project references to (any) other projects
…ferences/does not reference a package (by name)

An example use-case is something like the coverlet collector (https://github.com/coverlet-coverage/coverlet) which should be added ONLY to (ALL) test projects

- Added new SDK-style project & bumped project count
…property

Some examples:
- Convention.MustSetPropertyValue("Nullable", "enable") // Ensure nullable types are enabled for the project
- Convention.MustSetPropertyValue("IsPackable", "true") // Ensure the project can be packed for NuGet

See https://learn.microsoft.com/en-us/visualstudio/msbuild/property-element-msbuild?view=vs-2022#example
@flakey-bit flakey-bit force-pushed the feature/GH-87-new-conventions branch from 34559b1 to f5a4b38 Compare January 6, 2024 03:20
@flakey-bit flakey-bit marked this pull request as ready for review January 6, 2024 03:22
Copy link
Owner

@andrewabest andrewabest left a comment

Choose a reason for hiding this comment

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

An excellent changeset 👌 I don't have any feedback to address on this one. The implementations look good, the test coverage looks good, and the code follows the codebases conventions... 😉 Thankyou for the contribution!

@andrewabest
Copy link
Owner

Fixes #87

@andrewabest andrewabest merged commit 451c3bd into andrewabest:master Jan 7, 2024
1 check passed
@andrewabest
Copy link
Owner

Ah... just forgot about that sneaky region before hitting merge! I'll remove it @flakey-bit.

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