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: remove build-dashboard action from PR flow #3764

Merged
merged 4 commits into from
Jan 25, 2023

Conversation

jmayclin
Copy link
Contributor

@jmayclin jmayclin commented Jan 17, 2023

issue here: #3763. This action is flaky, likely due to a race condition between the doxygen generation and this action.

This commit removes the workflow from the PR flow, and instead schedules it as a cronjob which will run on each weekday at 00:00.

Resolved issues:

Partially addresses #3763

Description of changes:

This should prevent the irritating scenario of a merged PR being retroactively declared bad because of a failing pr-close action. Also threw in a git pull for some extra synchronization.

Call-Outs

I'd be in favor of doing this for the documentation generation as well, but that one doesn't seem nearly as flakey so I have less strong opinions on that.

issue-dashboard looks to be no longer maintained, so we should be on the lookout for a replacement on that one.

Testing:

no testing, ci alteration.

Revisions

Initial PR totally nuked this, turns out that was overkill because the node 12 deprecation is just a warning and not an error. In my defense, navigating around github's different actions and their corresponding failures can be a bit confusing.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

issue here: aws#3763. This action does
not run, since the issue-dashboard relies on node 12, which is
deprecated and no longer runs in GH actions.

This commit entirely removes the workflow. Once the team has a solution
this commit will likely be reverted and the action added back in, but
rather than just temporarily disabling it I think it's cleaner to just
entirely remove it while it's broken.
@jmayclin jmayclin changed the title ci: remove flaky build-dashboard action ci: remove build-dashboard action from PR flow Jan 18, 2023
I read the message wrong. The node deprecation thing is just a warning
and not an error. Thus we don't have to delete the whole thing. I still
hate the idea of note knowing whether an action is going to succeed
before I click merge though, so now going with this appraoch. Honestly,
I'd like to do the same thing for our documentation, but probably going
to get more pushback on that.
@jmayclin
Copy link
Contributor Author

https://github.com/aws/s2n-tls/pulls?q=is%3Apr+is%3Aclosed+

It looks like about 1/2 of the build dashboard actions fail

@dougch
Copy link
Contributor

dougch commented Jan 23, 2023

Cause for the failures #3775

@jmayclin jmayclin enabled auto-merge (squash) January 24, 2023 00:08
@dougch
Copy link
Contributor

dougch commented Jan 25, 2023

Ok, short term this will work, but the dashboards freshness does down. A better solution from #3775 is to use an action to update the gh-pages branch, which we can tackle next.

@jmayclin jmayclin merged commit 1a9bdb2 into aws:main Jan 25, 2023
knightjoel pushed a commit to knightjoel/s2n-tls that referenced this pull request Jan 25, 2023
issue here: aws#3763. This CI action 
frequently fails, resulting in a noisy merge history that means half of 
the merged PR's have a "checks failed" status.

As a first step, this commit removes the build-dashboard action from 
the PR workflow and instead schedules it to run once each weekday. 
Further actions are tracked in the linked issue.
@jmayclin jmayclin deleted the remove-build-dashboard branch December 22, 2023 01:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants