Skip to content

Conversation

gcurtis
Copy link
Collaborator

@gcurtis gcurtis commented Sep 6, 2022

Summary

The tests workflow only runs on PRs, so we were showing the status of whichever PR was built last. Use the release workflow instead, which only runs on main and includes the test workflow.

How was it tested?

N/A

The tests workflow only runs on PRs, so we were showing the status of
whichever PR was built last. Use the release workflow instead, which
only runs on main and includes the test workflow.

Signed-off-by: Greg Curtis <greg.curtis@jetpack.io>
@gcurtis gcurtis requested review from loreto and savil September 6, 2022 17:08
@loreto
Copy link
Contributor

loreto commented Sep 6, 2022

I'm tempted to just remove it: I've noticed that it renders the badge as 'red' if a PR on a branch fails, even if that branch is not merged with main, and main is still "green". Thoughts?

@savil
Copy link
Collaborator

savil commented Sep 6, 2022

I'm tempted to just remove it: I've noticed that it renders the badge as 'red' if a PR on a branch fails, even if that branch is not merged with main, and main is still "green". Thoughts?

@loreto isn't the intent of the PR to make the badge be "green" when main is working well, and avoid "red" status from PRs?

Copy link
Collaborator

@savil savil left a comment

Choose a reason for hiding this comment

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

approving based on my understanding that this PR actually addresses @loreto's concern

@mikeland73 mikeland73 merged commit 2101c18 into main Sep 6, 2022
@mikeland73 mikeland73 deleted the gcurtis/readme branch September 6, 2022 17:20
@gcurtis
Copy link
Collaborator Author

gcurtis commented Sep 6, 2022

@loreto yeah, that's what this is attempting to fix. It'll show the latest release build status instead. You can also include ?branch=main if we ever run the release workflow on other branches.

@loreto
Copy link
Contributor

loreto commented Sep 6, 2022

Let's include ?branch=main just to be safe?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants