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

Check for org pages repo matches unexpectedly #39

Closed
electrofelix opened this issue Apr 22, 2023 · 2 comments · Fixed by #40
Closed

Check for org pages repo matches unexpectedly #39

electrofelix opened this issue Apr 22, 2023 · 2 comments · Fixed by #40
Labels
bug Something isn't working

Comments

@electrofelix
Copy link
Contributor

I'm using this action in https://github.com/vagrant-libvirt/vagrant-libvirt, unfortunately because the repo name matches the org name, the current behaviour of identifying repos for updating org gh-pages repos things that previews for the main application which should show up under https://vagrant-libvirt.github.io/vagrant-libvirt/ as though they should appear under https://vagrant-libvirt.github.io/

This can be seen with vagrant-libvirt/vagrant-libvirt#1733 (comment) which indicates the preview should appear as https://vagrant-libvirt.github.io/pr-preview/pr-1733/ when the correct URL is https://vagrant-libvirt.github.io/vagant-libvirt/pr-preview/pr-1733/

It appears this was caused by an improvement to try and generate the correct preview URLs for PRs in repos updating org pages

org=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 1)
thirdleveldomain=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 2 | cut -d "." -f 1)
if [ ! -z "$customurl" ]; then
pagesurl="$customurl"
elif [ "$org" == "$thirdleveldomain" ]; then
pagesurl="${org}.github.io"

It appears that the code behaviour is expecting to match the following scenario:

GITHUB_REPOSITORY="vagrant-libvirt/vagrant-libvirt.github.io"

org=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 1)
thirdleveldomain=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 2 | cut -d "." -f 1)
[ "$org" == "$thirdleveldomain" ] && echo "${org} == ${thirdleveldomain}"

Unfortunately it will also match on the following:

GITHUB_REPOSITORY="vagrant-libvirt/vagrant-libvirt"

org=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 1)
thirdleveldomain=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 2 | cut -d "." -f 1)
[ "$org" == "$thirdleveldomain" ] && echo "${org} == ${thirdleveldomain}"

A simple fix should be to skip removal of the dot domain components:

org=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 1)
thirdleveldomain=$(echo "$GITHUB_REPOSITORY" | cut -d "/" -f 2)
[ "${org}.github.io" == "$thirdleveldomain" ] && echo "${org} == ${thirdleveldomain}"

will correctly skip outputting when GITHUB_REPOSITORY="vagrant-libvirt/vagrant-libvirt" while still detecting GITHUB_REPOSITORY="vagrant-libvirt/vagrant-libvirt.github.io"

electrofelix added a commit to electrofelix/pr-preview-action that referenced this issue Apr 22, 2023
When the repository matches the org name, which might be slightly more
common for small OSS projects where the owner wishes to move to using an
org to allow it be managed by other contributors over time, the existing
code will assume this means it is for the org level pages.

Consequently updates to a repo such as vagrant-libvirt/vagrant-libvirt
will have it's preview URL using the org style rather than the
repository style.

This change adjust the matching to limit to only allowing repos of the
name `<org>.github.io` to be presented with the org style gh-pages
preview URL.

Fixes: rossjrw#39
@rossjrw rossjrw added the bug Something isn't working label Apr 22, 2023
@rossjrw
Copy link
Owner

rossjrw commented Apr 22, 2023

Makes perfect sense to me and looks like a far better implementation of the .github.io detection. Will merge right away, thanks for the contribution!

@rossjrw
Copy link
Owner

rossjrw commented Apr 22, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants