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

ci: Fix conditions for building and publishing #313

Merged
merged 2 commits into from
Oct 28, 2022

Conversation

AbdealiLoKo
Copy link
Contributor

Based ont he github documentation:

github.events.ref: Is the ref of a release that has been created github.ref: If the branch that is being used (depending on the action)

  • create a release: It will be refs/tags/<tag_name>
  • create a pull request: It will be refs/pull/<pr_number>/merge
  • push to a branch: It will be refs/heads/<branch_name>

For publishing to pypi, we are targeting the "create a release" event. In which case:

  • github.events.ref will be refs/tags/<tag_name>
  • github.ref will also be refs/tags/<tag_name>

So, it looks like our IF condition was incorrect.

Ref: https://docs.github.com/en/actions/learn-github-actions/environment-variables

Based ont he github documentation:

github.events.ref: Is the ref of a release that has been created
github.ref: If the branch that is being used (depending on the action)
 - create a release:      It will be refs/tags/<tag_name>
 - create a pull request: It will be refs/pull/<pr_number>/merge
 - push to a branch:      It will be refs/heads/<branch_name>

For publishing to pypi, we are targeting the "create a release" event.
In which case:
 - github.events.ref will be refs/tags/<tag_name>
 - github.ref will also be refs/tags/<tag_name>

So, it looks like our IF condition was incorrect.

Ref: https://docs.github.com/en/actions/learn-github-actions/environment-variables
@JessicaTegner
Copy link
Owner

@AbdealiJK this seems very strange.

  • Building and publishing our latest version, should happen on the master branch, regardless if a tag is specified or not.
  • If a tag is specified that starts with "v" and we are on a master branch, we should do a release to pypi.

From what I can see from your suggested changes, this does not seem to be the case anymore?

@AbdealiLoKo
Copy link
Contributor Author

I don't think github actions has a way of detecting a tag is specified that starts with "v" and we are on a master branch (atleast not with the variables we are using right now.

From what I read in the documentation ...

The variables we are using are:

github.ref doc

The fully-formed ref of the branch or tag that triggered the workflow run. For workflows triggered by push, this is the branch or tag ref that was pushed. For workflows triggered by pull_request, this is the pull request merge branch. For workflows triggered by release, this is the release tag created. For other triggers, this is the branch or tag ref that triggered the workflow run. This is only set if a branch or tag is available for the event type. The ref given is fully-formed, meaning that for branches the format is refs/heads/<branch_name>, for pull requests it is refs/pull/<pr_number>/merge, and for tags it is refs/tags/<tag_name>. For example, refs/heads/feature-branch-1.

github.event doc

The full event webhook payload. You can access individual properties of the event using this context. This object is identical to the webhook payload of the event that triggered the workflow run, and is different for each event. The webhooks for each GitHub Actions event is linked in "Events that trigger workflows." For example, for a workflow run triggered by the push event, this object contains the contents of the push webhook payload.

and the 2 events we enabled in the yaml file are:

Leaving pull request aside for now ...

Push can have 2 cases:

  • Push a tag
  • Push a branch

The github.event.ref and github.ref will always be same for the "push"
event. So, we don't need to use github.event.ref in general.
@AbdealiLoKo
Copy link
Contributor Author

AbdealiLoKo commented Oct 28, 2022

When I am reading through the documentation again, I am realizing that we don't have a "create a release" event. We have a "push to tag" event

And github.ref and github.event.ref will always be the same for the push event. (both branch and tag)

So, tweaking your logic:

  • Building and publishing our latest version, should happen on any push to the master branch, regardless if a tag is specified or not.
  • If a tag is specified that starts with "v" and pushed, we should do a release to pypi.

In both events - push to master or version-tag we need to build the wheels

@JessicaTegner
Copy link
Owner

yes I guess that's the best we can do.
It's just strange, cause the current logic did work on v1.9 🤔

@AbdealiLoKo
Copy link
Contributor Author

AbdealiLoKo commented Oct 28, 2022

I believe the issue was introduced in where the IF conditions for both PR and Publish cases were being written
ad520d1#diff-944291df2c9c06359d37cc8833d182d705c9e8c3108e7cfe132d61a06e9133ddR73

In v1.9 from what I see, there were 2 separate yaml files being used each with their own on: criteria - which was merged into a single yaml file after the release 7ccd216

@JessicaTegner
Copy link
Owner

Do you think the current implementation you've written here, will work?

@AbdealiLoKo
Copy link
Contributor Author

I believe so
But I am not an expert at GitHub actions. So the only way to know is to try it out :)

@JessicaTegner JessicaTegner merged commit 1e118de into JessicaTegner:master Oct 28, 2022
@JessicaTegner
Copy link
Owner

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