In a large C# solution with many projects how do you maintain and enforce consistent project settings for the solution as a whole?
I work in a million+ line codebase with 50+ projects. We also have 60 developers. At any given time there are multiple features being worked on, bugs fixed, etc. While we have a culture of rigorous code reviews and feedback, changes to project files sneak in occasionally. Project settings are defined in a "cookbook" which describes how to create a new project from scratch, including all the custom build targets and settings that we require. The checkin would then be reviewed by a team lead and/or architect. Fixes (or changes) to our templates are checked in in an ad-hoc fashion, with the cookbook being updated accordingly.
The reliance on code reviews to catch errors in settings was not optimal. It put a lot of load on the reviewers to know what the current settings should be. Additionally, while the project files do need to be modified when files are added/removed, it is extremely easy to miss other types of changes. Due to the add/remove updates that are necessary for projects using the "old" csproj file, it wasn't possible to just prevent modifications at all. It was becoming evident that we needed a better, more consistent mechanism to enforce settings, and a watchdog to alert the team (or better yet, the developer that introduced it) when a setting was changed from the expected value. By coincidence, the team had just implemented a checkin gate on our CI build. Every build would run our Unit and Integration tests, only commiting the changeset to the VCS if all tests passed. The team decided that notifying someone when a setting changed was insufficient. If a project file was changed in such a way that it no longer matched our template the CI process should reject the checkin.
Enter the uncreatively named unit test "ProjectFileSettingsAreCorrect". In C# the .csproj files are just XML. We modified our build process to provide (via an environment variable) the location of the source files on disk. The test picks up this location, then finds all the .csproj files recursively. We then load the files one at a time, verifying that they contain only the expected build targets (Visual Studio has a habit of adding new build targets to all projects in the solution, especially while adding a new project.) After the build targets are verified the test then goes through their settings, verifying they have the expected settings.
The unit test has been live for almost 10 months now. Due to it being part of the checkin gate it isn't possible to accidentally change or introduce an incompatible setting. This frees up reviewers to focus more on the content of a changeset, which means they spend more time on the useful part of a code review. Review comments centered on project file changes have effectively disappeared.
The team has applied this principal to several other types of common mistakes we see in review comments. For example, we now verify files listed in the csproj exist in our VCS. We also verify the reverse, that code files under our source tree exist in the project file (we have a standing rule that if the file isn't included in the compilation, it should be deleted). This was a common mistake when deleting a file from the Visual Studio Solution Explorer versus deleting it from the Source Control Explorer. We verify that projects are using ProjectReferences instead of DllReferences where appropriate, etc.
The team is also adopting a policy of "automating review comments", IE, any review comment that can be programmatically enforced should be. In addition to these unit tests, we also use custom Roslyn Diagnostic Analyzers to provide feedback immediately to the developer, within the context of the error. (Analyzers have the added benefit of optionally providing a suggested fix inline).
Top comments (0)