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

Fix documentation deployment #35356

Merged
merged 1 commit into from
Apr 6, 2023
Merged

Fix documentation deployment #35356

merged 1 commit into from
Apr 6, 2023

Conversation

tobiasdiez
Copy link
Contributor

📚 Description

The update #35184 broke the docs upload. Hopefully this is fixed with this PR.

📝 Checklist

  • The title is concise, informative, and self-explanatory.
  • The description explains in detail what this PR is about.
  • I have linked a relevant issue or discussion.
  • I have created tests covering the changes.
  • I have updated the documentation accordingly.

⌛ Dependencies

@dimpase
Copy link
Member

dimpase commented Mar 26, 2023

OK, let's wait for CI to run its course

@tobiasdiez
Copy link
Contributor Author

Since by design, github only runs the workflow of the main branch of the repo (and not of the PR), this doesn't work here. However, I've tested it in my private repo and there the workflow is green now: https://github.com/tobiasdiez/sage/actions/runs/4540608136/jobs/8001729743.

Copy link
Collaborator

@kwankyu kwankyu left a comment

Choose a reason for hiding this comment

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

Okay, then.

@kwankyu
Copy link
Collaborator

kwankyu commented Mar 28, 2023

@vbraun

This is urgent.

@tornaria
Copy link
Contributor

tornaria commented Apr 6, 2023

Maybe it'd make sense to have an exception allowing workflow fixes like this one to be pushed to develop?

There are right now two critical PRs that are necessary for checks to pass: this one and #35418. The second one can be merged to a PR so checks are fixed, but this one cannot, as explained in #35356 (comment).

It's quite unfortunate when checks are not reliable, since it leads us to learn to ignore failures. Right now the last PR with checks passing is #35352, so that's a lot of PRs without any chance at passing checks.

Some ideas for the future:

  • maybe releases could go throught a PR before being pushed to devel so there is confirmation that all checks pass.
  • PRs changing workflows need a special approval/testing since its changes are not tested by the normal CI.
  • an exception could be made for a small PR to be pushed to devel without waiting for a release if it is necessary to get checks to pass.

I would argue that both this one and #35418 should be pushed to devel right now. This one is clear as it only touches workflows. The other one is debatable, but the alternative is every PR merging #35418 and if one doesn't know or forgets checks will fail.

@vbraun

@vbraun vbraun merged commit d051998 into develop Apr 6, 2023
@mkoeppe mkoeppe added this to the sage-10.0 milestone Apr 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants