fsprojects / FSharpx.Collections

FSharpx.Collections is a collection of datastructures for use with F# and C#.
http://fsprojects.github.io/FSharpx.Collections/
Apache License 2.0
247 stars 78 forks source link

GithubActions: enable for any branch #179

Closed knocte closed 1 year ago

knocte commented 3 years ago

When someone forks FSharpx.Collections to make a contribution, it's very likely that this person will push to a branch different than master (and make a PR from that branch). Or even if the aim is not a PR, there's no reason for not checking if the build passes on branches different than master (the nuget upload check already has a way to check if it's being uploaded from the master branch).

sergey-tihon commented 3 years ago

it's very likely that this person will push to a branch different than master (and make a PR from that branch).

Yes, that is how it works not. It runs on every commit in master or any PR in master (no matter what you branch name is). You can see it in you other PR, it was not executed for you automatically because you are first-time contributor (some new GitHub feature)

image

Or even if the aim is not a PR, there's no reason for not checking if the build passes on branches different than master

With your configuration Github Actions will be executed twice. Let's say I create new branch in the repo dev and open PR from dev to master to track the progress. Each new commit to dev trigger 2 GitHub action runs (because it is push and because it is pull_request). Current configuration does not have this issue.

the nuget upload check already has a way to check if it's being uploaded from the master branch

Current configuration it is not perfect either. Because of you fork repo, commit in master and then one pull request from master it also trigger GitHub Actions twice and windows-latest build of run triggered by push execute Publish NuGet steep for CI packages and fail because GITHUB_TOKEN in fork repo doe not have permissions to push nuget packages inside fsproject feed.

I do not yet know good solution for this and decided to merge #177 faster to unlock other PRs.

knocte commented 3 years ago

With your configuration Github Actions will be executed twice

There's nothing wrong with that. It would be executed first in my branch, and it gives me valuable information to know if I should open a PR out of it or not (if build is red, I might want to keep working on it until I create a PR).

... fail because GITHUB_TOKEN in fork repo doe not have permissions to push nuget packages inside fsproject feed.

The solution I've used in repos of mine to this problem is to check if GITHUB_TOKEN is defined before uploading (not just checking if branch is master).

knocte commented 3 years ago

The solution I've used in repos of mine to this problem is to check if GITHUB_TOKEN is defined before uploading (not just checking if branch is master).

Sorry, by GITHUB_TOKEN you mean NUGET_API_KEY? If yes, then my answer applies.

gdziadkiewicz commented 1 year ago

I agree with Knocte that it is annoying that I need to change the workflow config on my fork in order to make it build my feature branches. I also like the idea of checking NUGET_API_KEY to avoid errors. I will allow myself to resolve the conflicts, do some testing on my fork and if they pass merge this.

knocte commented 1 year ago

I also like the idea of checking NUGET_API_KEY to avoid errors.

I was willing to do this myself if the maintainer had agreed to, but I never heard back. Let me know if you prefer me to rebase from my side and finish the PR this way.

gdziadkiewicz commented 1 year ago

Please rebase and work on it

knocte commented 1 year ago

@gdziadkiewicz done

gdziadkiewicz commented 1 year ago

checking

gdziadkiewicz commented 1 year ago

Building your change on my fork's GitHub Action https://github.com/gdziadkiewicz/FSharpx.Collections/actions/runs/3708392238