Skip to content

Release build workflow #797

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.

Sign up for GitHub

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

Merged
dtchepak merged 3 commits into from
Oct 26, 2024
Merged

Release build workflow #797

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6971c00
Release build workflow
dtchepak Apr 20, 2024
67c0d29
Release build - apply review comments
dtchepak Apr 29, 2024
b13091c
Change artifact retention on release build
dtchepak Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .github/workflows/release_build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
name: Build release packages and documentation
on:
workflow_dispatch:
push:
Comment thread
dtchepak marked this conversation as resolved.
tags:
- 'v*'

env:
Comment thread
dtchepak marked this conversation as resolved.
CONFIGURATION: Release

jobs:
build:
runs-on: windows-latest
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0

- name: Setup .NET
uses: actions/setup-dotnet@v4
with:
dotnet-version: |
6.0.x
7.0.x

@Romfos Romfos Apr 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can remove .NET 7 sdk from this list

@304NotModified 304NotModified Apr 29, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

.NET 7 is still used here.

NSubstitute/tests/NSubstitute.Acceptance.Specs/NSubstitute.Acceptance.Specs.csproj

Line 4 in 19b38fd

<TargetFrameworks>net8.0;net7.0;net6.0;net462</TargetFrameworks>

Removing this is a breaking change I guess

@Romfos Romfos Apr 29, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

no, we don't need to run test for packing. we use for tests another pipeline

@304NotModified 304NotModified Apr 29, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Good one :)

@dtchepak dtchepak Apr 30, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Current targets have test as dependency so will still run. Can do this if we switch to dotnet pack

@304NotModified 304NotModified Apr 30, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Current targets have test as dependency so will still run. Can do this if we switch to dotnet pack

Maybe that's indeed better, but IMO is not that important. I don't mind the .NET 7 here

8.0.x

- name: Setup Ruby for documentation build
uses: ruby/setup-ruby@v1
with:
ruby-version: '3.2'
bundler-cache: true

- name: Build package and docs
run: dotnet run --project 'build/build.fsproj' -- -t All

@Romfos Romfos Apr 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I propose to use dotnet pack for packaging instead of custom tool and use tool only for documentation

- name: Pack
  run: dotnet pack -c Release -p:Version=${{ inputs.Version }} -o "bin/Release/NSubstitute" NSubstitute\NSubstitute.csproj

- name: Validate and build documentation
  run: dotnet run build/build.fsproj -- -t Documentation

@Romfos Romfos Apr 28, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

also as I see we have following fragment in csproj

  <PropertyGroup Condition="'$(CI)' == 'True'">
    <!--Deterministic Build and Source Link settings -->
    <ContinuousIntegrationBuild>true</ContinuousIntegrationBuild>
    <EmbedUntrackedSources>true</EmbedUntrackedSources>
    <IncludeSymbols>true</IncludeSymbols>
    <SymbolPackageFormat>snupkg</SymbolPackageFormat>
  </PropertyGroup>

maybe also make sense to add /p:CI=True or remove this condition from PropertyGroup (probably better to remove)
but I'm not an expert in snupkg and don't know how it works

@304NotModified 304NotModified Apr 29, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You can't do ContinuousIntegrationBuild locally and those settings are needed for deterministic packages/source link. Therefore the check on CI

@dtchepak dtchepak Apr 30, 2024

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I don't disagree with this, but do you have an outline of the pros and cons of switching?

The current FAKE-based build sorts out our versioning so I don't think it's as straightforward as switching directly (given constraint that I'd like to make version based on repo contents rather than an input).

Maybe can defer until larger build changes are done? (as per #797 (comment))

@Romfos Romfos Apr 30, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yes, 100% it could be changed later.

My private opinion:
dotent cli is a default way to interact with .NET projects like build\restore\pack\publish. From my point of view, make sense to use default approach for default problems. Just no need to reinvent the wheel. Fake and fsharp is just additional complexity.


- name: Upload packages
uses: actions/upload-artifact@v4
Comment thread
dtchepak marked this conversation as resolved.
with:
name: packages
path: |
bin/Release/NSubstitute/*.nupkg
bin/Release/NSubstitute/*.snupkg

- name: Upload documentation
uses: actions/upload-artifact@v4
with:
name: docs
path: |
bin/Release/nsubstitute.github.com/