Skip to content

Conversation

ktlim
Copy link
Contributor

@ktlim ktlim commented Mar 8, 2022

No description provided.

@ktlim ktlim force-pushed the tickets/DM-33936 branch from ebb8fa6 to 0dfb1bd Compare March 9, 2022 03:33
Copy link
Member

@kfindeisen kfindeisen left a comment

Choose a reason for hiding this comment

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

Looks good. Sorry for the delay; it took me a while to understand some bits.

Comment on lines 5 to 13
branches:
- main
paths:
- '.github/workflows/build-service.yml'
- 'python/activator/**'
pull_request:
paths:
- '.github/workflows/build-service.yml'
- 'python/activator/**'
branches:
- main
Copy link
Member

Choose a reason for hiding this comment

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

Why have the build be on PR, but not on push? This will make it harder to test changes to the activator code (the main reason I can imagine wanting branch builds in the first place).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's on any update to the PR, including pushes to the branch that is being PRed. Pushes to non-PR branches are more likely to be in-development and not actually ready for testing.

Copy link
Member

Choose a reason for hiding this comment

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

I don't follow. Isn't "in development" exactly when you want to test, while a PR means it's ready for use?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I typically use non-PR (or draft PR) branches as places to save random commits that may not actually function, creating the PR only when I expect it to actually work.

But the workflows as written allow you to manually trigger a build on any branch (or ref) you'd like: Actions tab, "Select workflow" button, "Run workflow" button, enter branch/tag/ref.

@ktlim ktlim force-pushed the tickets/DM-33936 branch from d551ffb to 5dc5117 Compare March 10, 2022 04:40
@ktlim
Copy link
Contributor Author

ktlim commented Mar 10, 2022

Urgh. Except that we've never pushed a "latest" to GitHub. Let me see if I can fix that...

@ktlim ktlim force-pushed the tickets/DM-33936 branch from 5dc5117 to 81c84b3 Compare March 10, 2022 05:21
@ktlim ktlim merged commit 3204204 into main Mar 10, 2022
@ktlim ktlim deleted the tickets/DM-33936 branch March 10, 2022 05:29
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