Discussion: Folder structure, SLN location, etc.

[Eilon]

The current template has this structure:

• src/
  • Library.sln
  • Library/
  • Library.Tests/

This is certainly fine for many projects, but when I tried using this for a project, I ran into some limitations when I had some additional scenarios. For example:

1. My project has more than one library
2. My project has an associated samples/ folder at the root that demonstrates usage of the library

My project structure is more like this:

• OverallProject.sln ◀ SLN is in the root and has a more generic name (matches the name of the repo, actually) so that it includes sources, tests, and samples
• samples/ ◀ This folder name is a well-known GitHub folder name that causes it to be excluded from Lines of Code counts, but it also just a friendly name. It needs to be built at some point in the build pipeline to ensure it restores/compiles, but produces no artifacts (similar to tests, but these aren't actual tests).
  • Library1_Sample/ ◀ Various samples folders go here (they are potentially somewhat arbitrary, and have no meaningful namign convention)
  • Library2_Sample/
• src/
  • Library1/
  • Library1.Tests/
  • Library2/ ◀ Another library + tests here
  • Library2.Tests/

I'm opening this issue to discuss whether this template should lay out some structure that makes it easier to have samples and easier to have more than one library, or any other thoughts anyone has on the folder structure.

[AArnott]

Thanks for sharing. I didn't know that github had special treatment for a samples folder.

It doesn't look like your proposal changes anything (aside from perhaps the sln file name, but then we couldn't name it after your repo since we don't know what your repo will be named yet. But VS makes renaming the sln easy so that seems like a low pri anyway.)

So is your suggestion here just additive (to discuss a samples folder)? Do you feel we need to define a sample as well just to suggest it or can folks add it if/when they want it?

[AArnott]

Something you didn't bring up here but I do want to discuss (and I think @bretjohnson briefly mentioned in our meeting) is whether tests should be under src or under a top-level tests folder. I generally prefer to keep test projects very "near" the projects they're testing, so I prefer the layout I have (and that you proposed as well, @eilon).

However with the advent of .editorconfig and in particular the deprecation of .ruleset files in favor of setting rule severity in .editorconfig, I'm questioning my pattern. With .ruleset files every project can point to either shipping.ruleset or tests.ruleset. But with .editorconfig it's strictly directory hierarchical based, which leans heavily in putting all tests under a single folder.

Now whether we use src\tests or just a top-level tests folder would still be a question.

[Eilon]

Oh you're right my proposal above is not a big change at all 😄 I forgot to make one change: I meant to propose moving the SLN to the root of the repo, so that it can more naturally include samples and src/tests. Of course an SLN can include anything from anywhere, but it seems more natural to have an SLN in the outer folder.

Moving the SLN to the root causes more things to require updating:

1. The Azure Pipelines stuff needs to be updated to build a bit differently
2. What gets packed is a tiny bit different
3. It might make sense to have additional Directory.targets/props in the samples folder
4. Probably some other stuff

These are all fairly minor changes, but when I adapted Library.Template into my existing project, I had to fuss with several configuration bits to make it work.

[Eilon]

As far as test location: I have no strong opinion. Nearness is nice, but so is hierarchy based on project type.

[AArnott]

Yes, I've wanted to move the .sln file to the root of the repo for quite some time. I've held back because if I do, VS will default to adding new projects to the wrong folder (the top level one) instead of the src folder.

What gets packed is a tiny bit different

How does the location of the sln impact what gets packed?

[AArnott]

I just filed a suggestion to get the .sln location issue fixed. Vote it up.

[Eilon]

How does the location of the sln impact what gets packed?

Oh not a huge difference, it's stuff that's easily controllable by Directory.props etc. For example we'd just need to ensure that the samples/ folder isn't packed with <IsPackable>false</IsPackable> or similar. Just the usual stuff about ensuring the right settings are used for each folder.

[Eilon]

Vote it up.

Done.

[AArnott]

@eilon they converted my feedback ticket to a suggestion (which I thought I had originally, but oh well). It shows 0 votes, so maybe your vote was lost in the transition. Care to vote again?

[Eilon]

Well that's lovely. Done.

[Eilon]

I updated the original issue text above to also reflect my desired location of the SLN (in the root) per the later discussion.

[SteveBush]

I wanted to echo Eilon's comments. I am definitely not saying below is best practices just how I've worked for a while.

Here's how I structured one of my platform solutions which builds and tests a set of platform packages across several platforms.

Solution.sln
eng
docs
setup
src
   core
       CoreProject1 (Any CPU)
   platform
       xamarin
           XamarinProject (shared)
       ios
           IOSProject (Xamarin.IOS)
       android
          Android (Xamarin.Android)  
       windows
          Windows
test
    platform
        XunitProjects
    core
        XunitProjects

I've found it helpful to separate the Solution root, src and test directories so I can reference different Directory.Build.Props for each. For example, I include FluentAssertions for the test directory. Each Directory.Build.Props includes a reference to it's parent for inheritance.

[AArnott]

I've been reconsidering a top-level test directory a little bit. Directory.Build.props is one reason, but a bigger one is .editorconfig, so that certain analyzers can be conveniently suppressed for all test projects. Both of these can be achieved in the current layout with one top level src folder, but with rulesets being deprecated in favor of .editorconfig, it's getting harder to make the case that this is the best structure. Also regarding a top-level sln file: certainly it makes more sense in a src+test world. But even without that, I put sln under src so that "Add New Project" in VS automatically put the project under src as well, but that's such an uncommon operation compared to building, opening, etc., that I think moving sln to top-level makes more sense now.

[AArnott]

In trying out the top level test directory approach, it occurred to me that the only way we could have a Directory.Build.props that applies to both product and test projects would be to put them in the repo-root. But that means they would also apply to samples if that was another top-level directory. I guess someone could add a directory.build.props to the samples directory that does not inherit from the repo root if that's what they wanted though.

[AArnott]

It does mean a lot more files in the repo root (at least 4 more). I like how the repo root is fairly clean -- it makes finding README, CONTRBUTING, LICENSE and other files a bit easier. Still experimenting.

[SteveBush]

Here's what I've done. My repo level Directory.Build.Props is below.

---

<Project>
  <Import Project="$(MSBuildThisFileDirectory)\eng\msbuild\Solution.Build.props" />

  <PropertyGroup>
    <PackageProjectUrl>https://github.com/PureActive/PureActive</PackageProjectUrl>
    <RepositoryUrl>https://github.com/PureActive/PureActive.git</RepositoryUrl>
    <PackageReleaseNotes>Bug fixes and performance improvements.</PackageReleaseNotes>
  </PropertyGroup>

</Project>

---

I have Solution.Build.Props under /eng/msbuild that has properties that are common to all of my solutions.

eng/msbuild.Solution.Build.props

<Project>

  <ItemGroup>
     <PackageReference Include="JetBrains.Annotations" Version="2019.1.3" />
    <PackageReference Include="Microsoft.SourceLink.GitHub" Version="1.0.0" PrivateAssets="All"/>
  </ItemGroup>

  <!-- Global Property Switches-->
  <PropertyGroup>
    <IncludeCodeAnalysis>false</IncludeCodeAnalysis>
  </PropertyGroup>

  <!-- Microsoft.CodeAnalysis.FxCopAnalyzers -->
  <!--<ItemGroup Condition="'$(IncludeCodeAnalysis)' == 'true'">
    <PackageReference Include="Microsoft.CodeAnalysis.FxCopAnalyzers" Version="2.9.8">
      <PrivateAssets>all</PrivateAssets>
      <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
    </PackageReference>
  </ItemGroup>-->

  <!-- SourceLink Properties -->
  <PropertyGroup>
    <!-- Don't Append Commit SHA to InformationalVersion.  GitVersionTask does this for us -->
    <IncludeSourceRevisionInInformationalVersion>false</IncludeSourceRevisionInInformationalVersion>

    <RepositoryType>git</RepositoryType>

    <!-- Optional: Publish the repository URL in the built .nupkg (in the NuSpec <Repository> element) -->
    <PublishRepositoryUrl>true</PublishRepositoryUrl>

    <!-- Optional: Embed source files that are not tracked by the source control manager in the PDB -->
    <EmbedAllSources>true</EmbedAllSources>

    <!-- Optional: Build symbol package (.snupkg) to distribute the PDB containing Source Link -->
    <IncludeSymbols>true</IncludeSymbols>
    <SymbolPackageFormat>snupkg</SymbolPackageFormat>

    <!--<RepoRoot>$(MSBuildThisFileDirectory)/../..</RepoRoot>-->
  </PropertyGroup>

  <!-- Packaging Properties -->
  <PropertyGroup>
    <LangVersion>latest</LangVersion>
    <PackageOutputPath>$([System.IO.Path]::Combine($(MSBuildProjectDirectory), 'packages'))</PackageOutputPath>
    <AllowedOutputExtensionsInPackageBuildOutputFolder>$(AllowedOutputExtensionsInPackageBuildOutputFolder);.xml;*.json</AllowedOutputExtensionsInPackageBuildOutputFolder>

    <Authors>Steve Bush</Authors>
    <Company>BushChang Corporation</Company>
    <Copyright>© 2020 BushChang Corporation. All rights reserved.</Copyright>
    <Owners>SteveBu</Owners>

    <PackageLicenseExpression>MIT</PackageLicenseExpression>
    <PackageRequireLicenseAcceptance>false</PackageRequireLicenseAcceptance>
  </PropertyGroup>

  <Target Name="FreshClean" BeforeTargets="Clean">
    <RemoveDir Directories="$(PackageOutputPath)" />
  </Target>

  <!-- Signing Properties -->
  <PropertyGroup>
    <SignAssembly>true</SignAssembly>
    <AssemblyOriginatorKeyFile>$(MSBuildThisFileDirectory)\..\keys\PureActive.snk</AssemblyOriginatorKeyFile>

    <!-- Remove Warning about reference assembly does not have a strong name -->
    <NoWarn>$(NoWarn);CS8002</NoWarn>
  </PropertyGroup>

</Project>

My long term goal was to have the /eng directory be hosted in a separate repository. It would contain all files, scripts, etc. shared by all of my solutions. Using the repositories feature of Azure Pipelines I could pull the eng from a shared build repository.

However, I just discovered your Template Library this morning and I plan to rewrite my build process based on it. You did a great job!

[AArnott]

Thanks, @SteveBush.

Everyone: Check out #46 for a top-level test folder and vote on it.