Skip to content

Improved CI/CD Pipelines - #19

Merged
lord-executor merged 5 commits into
mainfrom
feature/improved-ci-cd-pipelines
Aug 20, 2026
Merged

lord-executor merged 5 commits into
mainfrom
feature/improved-ci-cd-pipelines

Conversation

@lord-executor

@lord-executor lord-executor commented Aug 19, 2026

Copy link
Copy Markdown
Owner

Well... It's a LOT of changes, but mostly they are automated formatting fixes and dealing with build warnings from inconsistencies in the code, so it's not as bad as it looks.

Migration to SLNX Format

I've been working with *.slnx style solutions for a while now and they are simply... better. Readable. I updated the solution and I have no regrets.

Improved Build Pipeline and Semi-Automated Release

I extended and renamed the GitHub build pipeline with some security constraints and other improvements. Then I introduced a new publish.yml workflow that uses the brand new NuGet Trusted Publishing mechanism to automatically publish packages. I've been testing this on some of my other projects and I am much more comfortable actually automating this since it significantly reduces the risk of leaking long-living API keys. It also makes it much easier for any of the project maintainers to actually create a release since all you have to do is... push a tag - at least if I configured the repository Ruleset correctly. See doc\release-process.md.

Running Analyzers on Build

I enabled running code analyzers throughout the project with the src/Directory.Build.props which of course generated dozens of warnings because of small inconsistencies in the code. I disabled some of the warnings on purpose and simply dealt with the rest by fixing the code.

These are the warnings that I disabled:

  • CA1805 - disabled everywhere since I really don't find that a particularly useful rule. Sometimes it does make sense to explicitly initialize a variable to its default value.
  • CA1822 - disabled everywhere since this shows up quite a lot and I find it to be a bit of a "premature optimization". Just because some method does not access members right now, doesn't mean it should be static.
  • CA1707 - disabled for all testing projects since we do use underscores in test method names.
  • CA2263 - disabled for all testing projects because I simply did not feel like dealing with all of those warnings 😝

Consistent Formatting and Check in CI/CD

We've had a bit of a formatting mess in this project from the very beginning and since I was already working on the build process, I decided to finally do something about this. I updated the .editorconfig with some (according to me) sane rules and ran dotnet format src/Ninject.Web.AspNetCore.slnx to apply these consistent formatting rules. These rules are now also checked in the CI/CD pipelines and the build will fail if there are violations.

@lord-executor lord-executor self-assigned this Aug 19, 2026
branches: [ "main" ]

concurrency:
group: ${{ github.workflow }}-${{ github.ref }}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This means that if I push changes to a branch while a build is already running, then the current build will be aborted and a new one started.

cancel-in-progress: true

permissions:
contents: read

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Safeguard against... shenanigans.

- name: Test
run: dotnet test --no-build $SOLUTION_FILE

# Derive the package version from the pushed tag, e.g. refs/tags/v10.0.1 -> 10.0.1

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Yes, this also works for SemVer pre-release versions like "v10.2.0-beta.1" where the derived version will be "10.2.0-beta.1".

uses: NuGet/login@v1
id: login
with:
user: ${{ secrets.NUGET_USER }}

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Not exactly a secret since it's really just my NuGet username 😄. But apparently that's how it is done.

@@ -1,4 +1,4 @@
using Microsoft.AspNetCore.Hosting;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Many files have a BOM which always annoys the living F out of me. An UTF-8 BOM makes no F-ing sense.

namespace Ninject.Web.AspNetCore.Test.Fakes
{
public class FakeActivationCacheAccessor : IActivationCache, IActivationCacheAccessor
public sealed class FakeActivationCacheAccessor : IActivationCache, IActivationCacheAccessor

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This avoids the warning for calling GC.whatever... in the Dispose method which is one of the analyzer checks.

}

private IKernel CreateKernel()
private AspNetCoreKernel CreateKernel()

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Another analyzer warning.

public sealed class ActivationCacheTests : IDisposable
{
private readonly WeakTableActivationCache testee;
private readonly WeakTableActivationCache _testee;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This did not get disposed properly. Analyzer warnings can be useful 😝.

{
throw new ArgumentNullException(nameof(serviceCollection));
}
ArgumentNullException.ThrowIfNull(serviceCollection);

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Also recommended by an analyzer warning.

@DominicUllmann

Copy link
Copy Markdown
Collaborator

Thanks for these updates, looks good :-)

@lord-executor
lord-executor merged commit e004bba into main Aug 20, 2026
3 checks passed
@lord-executor
lord-executor deleted the feature/improved-ci-cd-pipelines branch August 20, 2026 06:07
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