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

Skip the 'build_html' job on repository forks #515

Merged

Conversation

vaitkus
Copy link
Contributor

@vaitkus vaitkus commented Apr 7, 2024

The 'build_html' job fails on repository forks that do not have the gh_pages branch. This PR modifies the job so it is only run on the original Materials-Consortia/OPTIMADE repo.

@vaitkus vaitkus requested a review from ml-evs April 7, 2024 02:11
rartino
rartino previously approved these changes Apr 8, 2024
Copy link
Contributor

@rartino rartino left a comment

Choose a reason for hiding this comment

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

This is better than the situation now (so approved). But, I wonder if it wouldn't be even better if possible to formulate it "if: a gh_pages branch exists" to make it possible for people to recover the main repo behavior (which can be useful also for forks) without having to add diverging commits to the repo.

@vaitkus
Copy link
Contributor Author

vaitkus commented Apr 8, 2024

I am not sure if the check for the branch existence can be carried out in the if part of the workflow. Maybe the new approach of exiting from the job with status 0 would work instead?

Copy link
Member

@ml-evs ml-evs left a comment

Choose a reason for hiding this comment

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

New version looks fine to me, the only real test will be merging it...

@vaitkus vaitkus requested a review from rartino April 8, 2024 13:56
Copy link
Contributor

@rartino rartino left a comment

Choose a reason for hiding this comment

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

Yes, lets merge this. But if we set up a separate repo for building and publishing the html version, this may end up not being used here anyway.

@rartino rartino merged commit 604468a into Materials-Consortia:develop Apr 9, 2024
4 checks passed
@vaitkus vaitkus deleted the skip-build-html-job-on-forks branch April 9, 2024 13:42
@vaitkus
Copy link
Contributor Author

vaitkus commented Apr 9, 2024

Unfortunately, it seems that any non-zero status terminates the job so the job still fails on forks without this branch. I think
rewriting to something like:

git ls-remote --exit-code --heads origin refs/heads/gh-pages > /dev/null || exit 0

should work, but I should probably test it first before submitting another PR.

@vaitkus
Copy link
Contributor Author

vaitkus commented Apr 9, 2024

Ok, this approach does not seem to work in general. Exiting from the "Commit to gh-pages" job with a zero status results in the "Push changes" job still getting executed (and thus creating a "gh-pages" branch on the fork, but without the HTML pages). I will leave this alone unless the separate repo approach does not work out.

@rartino
Copy link
Contributor

rartino commented Apr 10, 2024

Ah, the following steps would have to be updated as well to do nothing. So, should we revert this merge?

@vaitkus
Copy link
Contributor Author

vaitkus commented Apr 10, 2024

We probably should since it is a bit misleading (comments do not match the actual behaviour). Is there an easy built-in way to do so in GitHub?

@rartino
Copy link
Contributor

rartino commented Apr 10, 2024

The merge icon in this PR thread had a "Revert" button. I clicked that, and it created a revert PR.

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.

3 participants