jorgensd / adios4dolfinx

Extending DOLFINx with checkpointing functionality
http://jsdokken.com/adios4dolfinx/
MIT License
20 stars 7 forks source link

Do not deploy to GitHub pages on forks #42

Closed francesco-ballarin closed 11 months ago

francesco-ballarin commented 11 months ago

Forks typically do not have pages enabled (and even if they did it would probably be confusing for search engines to have several copies of the documentation on different *.github.io sites).

Failure before this PR: https://github.com/francesco-ballarin/adios4dolfinx/actions/runs/7083058047 Tested by temporarily changing the branch name from main to **: https://github.com/francesco-ballarin/adios4dolfinx/actions/runs/7083197628

jorgensd commented 11 months ago

Is there a compelling reason for working on a main branch in a fork? I would expect someone working on main on a fork to either:

  1. Want to branch off the project completely, and can then change the workflows as they want, maybe even host their own docs.
  2. Short lived forks for patches, where a failing deploy workflow doesn’t bother anyone. It is not a criterion for passing pull requests, and I wouldn’t expect it to be green to accept a PR.
francesco-ballarin commented 11 months ago

Is there a compelling reason for working on a main branch in a fork?

None, but one would have to sync their fork once in a while (and this would push to main) for creating new PRs, as I did this morning while creating this PR.

jorgensd commented 11 months ago

My rationale for not wanting this change is that the current workflow is as of now is fairly generic (with some assumptions on other workflows existing in the same repo). It makes it easy to copy paste the structure without any fear of missing something repo specific.

francesco-ballarin commented 11 months ago

Understood!