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

Trigger CI on every push #1011

Merged
merged 6 commits into from
Jan 26, 2022
Merged

Trigger CI on every push #1011

merged 6 commits into from
Jan 26, 2022

Conversation

randombenj
Copy link
Contributor

Requirements

All new code should be covered with tests, documentation should be updated. CI should pass.

Description of the Change

Triggers the CI build/test on every push (#1010 (comment)) as suggested by @ximon18

Checklist

  • unit-test added (if change is algorithm)
  • functional test added/updated (if change is functional)
  • man page updated (if applicable)
  • bash completion updated (if applicable)
  • documentation updated
  • author name in AUTHORS

@ximon18
Copy link
Contributor

ximon18 commented Jan 26, 2022

@randombenj: For some reason your "single" push triggered the workflow three times:

For the first two Git reports:

This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

I don't see anything in the documentation that explains this. However this may be why in the projects I have worked on the trigger for the push event was for pushes to the main branch only, and then also trigger on the pull_request event (either to any branch or just to main depending on the process used by the project).

Also note that one of the "benefits" (it can be confusing though) of triggering on the pull_request event is that the workflow is run as if the merge has already happened to the target branch, i.e. it tests against the result of merging the pull request, not against the code solely in the PR branch.

@lbolla
Copy link
Contributor

lbolla commented Jan 26, 2022

FYI, I've rebased molior-rebase against master, so this PR will need rebase against the new molior-rebase, too.

@randombenj
Copy link
Contributor Author

randombenj commented Jan 26, 2022

@ximon18 That's actually intended as I force pushed 2 times (and therefore the commit changed) :)

I would strongly suggest going for on: push as not only main and PRs are tested but every single push so you immediately notice when something breaks ;)

@randombenj
Copy link
Contributor Author

@lbolla rebased the changes

@lbolla
Copy link
Contributor

lbolla commented Jan 26, 2022

Builds for go 1.13 fail because of the "azure" library, which is using features from go 1.14+: https://pkg.go.dev/net/http#Header.Values

@lbolla
Copy link
Contributor

lbolla commented Jan 26, 2022

1.17 fails. I've opened a separate ticket: #1012

EDIT: 1.16 fails with the same error. Ticket updated.

@randombenj
Copy link
Contributor Author

@lbolla shall we reenable 1.17?

@randombenj randombenj merged commit dcb909d into molior-rebase Jan 26, 2022
@neolynx neolynx deleted the trigger-ci-on-push branch July 3, 2024 10:21
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.

None yet

3 participants