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

chore: Add support for GitHub Packages #173

Merged
merged 1 commit into from Jan 19, 2024

Conversation

austindrenski
Copy link
Member

@austindrenski austindrenski commented Jan 11, 2024

GITHUB_REF version format
refs/heads/* #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9}
refs/pull/* #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9}

See: #54, open-feature/dotnet-sdk-contrib#134

Closes: #54

Copy link

codecov bot commented Jan 11, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (f47cf07) 0.00% compared to head (0a0273d) 93.86%.

Additional details and impacted files
@@            Coverage Diff            @@
##           main     #173       +/-   ##
=========================================
+ Coverage      0   93.86%   +93.86%     
=========================================
  Files         0       23       +23     
  Lines         0      946      +946     
  Branches      0      105      +105     
=========================================
+ Hits          0      888      +888     
- Misses        0       34       +34     
- Partials      0       24       +24     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@beeme1mr beeme1mr requested a review from benjiro January 11, 2024 21:23
.github/workflows/ci.yml Outdated Show resolved Hide resolved
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 11, 2024
See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 11, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
@austindrenski
Copy link
Member Author

Well, still failed, but now I'm pretty sure it's just the fork issue. Assuming ${{ secrets.NUGET_TOKEN }} is available to others, then this will work as is, otherwise, I can update the conditions to skip publishing from forks.

@benjiro
Copy link
Member

benjiro commented Jan 11, 2024

FYI the secrets.NUGET_TOKEN is for nuget.org registry but looking at this change its pushing to github registry. Also i believe forks won't have access to any secrets from memory. You might need to update the permissions on the workflow to get write access to github registeries on the secrets.GITHUB_TOKEN that the action runner generates https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs#defining-access-for-the-github_token-scopes

One problem with using githubs nuget registry is you need a github account setup to pull down the packages (this could of change since last time i checked). This is why i originally suggested using something like MyGet which offer free accounts for OpenSource projects.

I think gating the prerelease packages behind auth defeats the purpose, and makes for a cumbersome developer experience. Happy to go a head with it if people are fine with that.

@austindrenski
Copy link
Member Author

FYI the secrets.NUGET_TOKEN is for nuget.org registry but looking at this change its pushing to github registry. Also i believe forks won't have access to any secrets from memory. You might need to update the permissions on the workflow to get write access to github registeries on the secrets.GITHUB_TOKEN that the action runner generates https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs#defining-access-for-the-github_token-scopes

Bah, I knew there was something I was forgetting. Thanks, will update.

One problem with using githubs nuget registry is you need a github account setup to pull down the packages (this could of change since last time i checked). This is why i originally suggested using something like MyGet which offer free accounts for OpenSource projects.

I think gating the prerelease packages behind auth defeats the purpose, and makes for a cumbersome developer experience. Happy to go a head with it if people are fine with that.

Full agree that GitHub should just give us the option to allow un-auth'd package retrieval for OSS projects, but I also don't think the auth requirement is all that big of a blocker.

Now this could be some corporate dev bias, but of all the authenticated feeds I have to deal with at my day job, GitHub Packages is the least annoying of them lol.

But bigger picture, I think providing prerelease packages directly from GitHub is a really useful solution for discoverability, plus if you're already setup to consume one GitHub package feed, then that auth carries over to the N+1 feed too.

So wouldn't ever recommend we only publish to GitHub Packages (e.g. as opposed to, say, nuget.org), but I think it's a really nice built-in option that comes pretty much for free (as in beer/time) and offers a nice UX in terms of code+packages+releases all in one place.

(also, none of what I'm saying here precludes also publishing to myget in addition to github, I'm just saying there's no reason to not offer github while we're at it.)

austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 12, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 12, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
@austindrenski austindrenski self-assigned this Jan 18, 2024
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 19, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

I think this is wrong. I could be missing something, but pull_request_target runs in the context of the base branch - so this will make all our CI tests run the code in main, not the code in whatever PR.

@austindrenski
Copy link
Member Author

I think this is wrong. I could be missing something, but pull_request_target runs in the context of the base branch - so this will make all our CI tests run the code in main, not the code in whatever PR.

Geeze, I think you're right. I'm not sure what I was doing when I changed that (testing token perms, maybe?).

Reverting now.

austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 19, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
Comment on lines +32 to +34
<ItemGroup Condition="'$(OS)' == 'Unix'">
<PackageReference Include="Microsoft.NETFramework.ReferenceAssemblies" Version="1.0.3" PrivateAssets="all" />
</ItemGroup>
Copy link
Member

@toddbaert toddbaert Jan 19, 2024

Choose a reason for hiding this comment

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

Is this because we're not building on Windows in the job? Will this have any impact on the release build (it does run on windows)?

Copy link
Member Author

Choose a reason for hiding this comment

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

Is this because we're not building on Windows in the job?

Yep! It gives dotnet build enough context to build the net462 TFM on Linux, so we can run package builds on ubuntu-latest which, among other reasons, runs circles around windows-latest for dotnet restore and anything else that uses compression:

  • windows-latest:
    Run dotnet test test\OpenFeature.Tests\ --configuration Release --logger GitHubActions
      Determining projects to restore...
      Restored D:\a\dotnet-sdk\dotnet-sdk\src\OpenFeature\OpenFeature.csproj (in 21.2 sec).
      Restored D:\a\dotnet-sdk\dotnet-sdk\test\OpenFeature.Tests\OpenFeature.Tests.csproj (in 1.69 min).
    
  • ubuntu-latest:
    Run dotnet test test/OpenFeature.Tests/ --configuration Release --logger GitHubActions
      Determining projects to restore...
      Restored /home/runner/work/dotnet-sdk/dotnet-sdk/src/OpenFeature/OpenFeature.csproj (in 5.33 sec).
      Restored /home/runner/work/dotnet-sdk/dotnet-sdk/test/OpenFeature.Tests/OpenFeature.Tests.csproj (in 5.5 sec).
    

Will this have any impact on the release build (it does run on windows)?

It should have no impact since it's conditioned on the OS var and the reference assemblies won't be included in the compilation, leaving dotnet to find the actual assemblies in the GAC (or on disk, can't remember which it actually checks, so don't quote me on this part).

But it does mean, that once everyone is comfortable with it (i.e. this workflow runs without issues a few times), we could then update release.yml to run on ubuntu-latest too.

That workflow obviously runs less frequently than ci.yml, so the dotnet restore perf won't be as big of a consideration in that case, but I personally feel better building my internal stable releases on the ubuntu-latest runners. Things just always seem to work better there lol (e.g. cli perf, line endings, etc).

edit: opened #195 for later

Copy link
Member

Choose a reason for hiding this comment

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

Makes sense. I'm in favor of that change in the future. Thanks for the explanation 🙏

@toddbaert toddbaert self-requested a review January 19, 2024 22:54
@toddbaert toddbaert dismissed their stale review January 19, 2024 22:55

recent changes

Copy link
Member

@toddbaert toddbaert left a comment

Choose a reason for hiding this comment

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

Approving again, and will merge in a moment, but consider this.

@toddbaert toddbaert merged commit 26cd5cd into open-feature:main Jan 19, 2024
11 checks passed
@austindrenski austindrenski deleted the add-github-packages branch January 19, 2024 23:33
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk that referenced this pull request Jan 19, 2024
Signed-off-by: Austin Drenski <austin@austindrenski.io>
@austindrenski
Copy link
Member Author

Woohoo, it lives! 🥳🥳🥳

image

austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 20, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 20, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 20, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 20, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 20, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 20, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 22, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk that referenced this pull request Jan 22, 2024
Follows open-feature#173 and open-feature/dotnet-sdk-contrib#134 to synchronize
ci.yml between both repos.

```console
$ diff dotnet-sdk{,-contrib}/.github/workflows/ci.yml

```

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk that referenced this pull request Jan 22, 2024
Follows open-feature#173 and open-feature/dotnet-sdk-contrib#134 to synchronize
ci.yml between both repos.

```console
$ diff dotnet-sdk{,-contrib}/.github/workflows/ci.yml

```

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk-contrib that referenced this pull request Jan 22, 2024
| GITHUB_REF    | version format                                   |
|---------------|--------------------------------------------------|
| refs/heads/*  | #.#.#-ci.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/pull/*   | #.#.#-pr.{%Y%m%d}T{%H%M%S}+sha.${GITHUB_SHA:0:9} |
| refs/tags/v*  | #.#.#                                            |

See: open-feature/dotnet-sdk#54, open-feature/dotnet-sdk#173

Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk that referenced this pull request Jan 23, 2024
Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit that referenced this pull request Jan 23, 2024
Signed-off-by: Austin Drenski <austin@austindrenski.io>
austindrenski added a commit to austindrenski/open-feature-dotnet-sdk that referenced this pull request Jan 27, 2024
Follows open-feature#173 and open-feature/dotnet-sdk-contrib#134 to synchronize
ci.yml between both repos.

```console
$ diff dotnet-sdk{,-contrib}/.github/workflows/ci.yml

```

Signed-off-by: Austin Drenski <austin@austindrenski.io>
askpt pushed a commit that referenced this pull request Jan 30, 2024
Signed-off-by: Austin Drenski <austin@austindrenski.io>
Signed-off-by: André Silva <2493377+askpt@users.noreply.github.com>
beeme1mr pushed a commit that referenced this pull request Mar 12, 2024
🤖 I have created a release *beep* *boop*
---


##
[1.5.0](v1.4.1...v1.5.0)
(2024-03-12)


### 🐛 Bug Fixes

* Add targeting key
([#231](#231))
([d792b32](d792b32))
* Fix NU1009 reference assembly warning
([#222](#222))
([7eebcdd](7eebcdd))
* invalid editorconfig
([#244](#244))
([3c00757](3c00757))


### ✨ New Features

* Flag metadata
([#223](#223))
([fd0a541](fd0a541))
* implement in-memory provider
([#232](#232))
([1082094](1082094))


### 🧹 Chore

* bump spec version badge
([#246](#246))
([ebf5552](ebf5552))
* cleanup unused usings 🧹
([#240](#240))
([cdc1bee](cdc1bee))
* **deps:** update actions/upload-artifact action to v4.3.0
([#203](#203))
([0a7e98d](0a7e98d))
* **deps:** update actions/upload-artifact action to v4.3.1
([#233](#233))
([cfaf1c8](cfaf1c8))
* **deps:** update codecov/codecov-action action to v3.1.5
([#209](#209))
([a509b1f](a509b1f))
* **deps:** update codecov/codecov-action action to v3.1.6
([#226](#226))
([a577a80](a577a80))
* **deps:** update dependency coverlet.collector to v6.0.1
([#238](#238))
([f2cb67b](f2cb67b))
* **deps:** update dependency fluentassertions to v6.12.0
([#215](#215))
([2c237df](2c237df))
* **deps:** update dependency microsoft.net.test.sdk to v17.8.0
([#216](#216))
([4cb3ae0](4cb3ae0))
* **deps:** update dependency nsubstitute to v5.1.0
([#217](#217))
([3be76cd](3be76cd))
* **deps:** update dependency openfeature.contrib.providers.flagd to
v0.1.8 ([#211](#211))
([c1aece3](c1aece3))
* **deps:** update xunit-dotnet monorepo
([#236](#236))
([fa25ece](fa25ece))
* Enable Central Package Management (CPM)
([#178](#178))
([249a0a8](249a0a8))
* Enforce coding styles on build
([#242](#242))
([64699c8](64699c8))
* More sln cleanup
([#206](#206))
([bac3d94](bac3d94))
* SourceLink is built-in for .NET SDK 8.0.100+
([#198](#198))
([45e2c86](45e2c86))
* Sync ci.yml with contrib repo
([#196](#196))
([130654b](130654b))
* Sync release.yml with ci.yml following
[#173](#173)
([#195](#195))
([eba8848](eba8848))


### 📚 Documentation

* fix hook ecosystem link
([#229](#229))
([cc6c404](cc6c404))
* update the feature table key
([f8724cd](f8724cd))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
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.

[FEATURE] Publish dev builds to ghcr nuget
4 participants