Skip to content
This repository was archived by the owner on May 1, 2025. It is now read-only.

ci: run on pull requests#439

Merged
lucaswerkmeister merged 2 commits into
wmde:masterfrom
lucaswerkmeister:ci
Jul 2, 2021
Merged

ci: run on pull requests#439
lucaswerkmeister merged 2 commits into
wmde:masterfrom
lucaswerkmeister:ci

Conversation

@lucaswerkmeister
Copy link
Copy Markdown
Member

Run CI on pull requests, including external ones. To avoid double CI runs, this means we no longer run CI on pushes, except to the master branch (after a pull request was merged). The deploy job is skipped on external pull requests, since the secrets won’t be available; this should cause the browser-tests job to be skipped as well.

Bug: T283911

Run CI on pull requests, including external ones. To avoid double CI
runs, this means we no longer run CI on pushes, except to the master
branch (after a pull request was merged). The deploy job is skipped on
external pull requests, since the secrets won’t be available; this
should cause the browser-tests job to be skipped as well.

Bug: T283911
@sai-san sai-san requested a review from a team June 29, 2021 10:44
@lucaswerkmeister lucaswerkmeister marked this pull request as ready for review July 2, 2021 08:30
@lucaswerkmeister lucaswerkmeister merged commit 5f29882 into wmde:master Jul 2, 2021
@lucaswerkmeister lucaswerkmeister deleted the ci branch July 2, 2021 08:36
deploy:
runs-on: ubuntu-latest
needs: setup
if: ${{ github.repository_owner == 'wikit' }}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this broke our CI. It should have run in #443, but it didn't.

Copy link
Copy Markdown
Member Author

@lucaswerkmeister lucaswerkmeister Jul 8, 2021

Choose a reason for hiding this comment

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

Wait, shouldn’t this be wmde? wikit is the repository name, not owner.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Let’s see if #444 fixes it.

@micgro42 micgro42 mentioned this pull request Jul 8, 2021
push:
branches:
- master
pull_request:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I'm not entirely sure yet, but I suspect this has also somehow effectively broken the Netlify link that is commented into the PRs. I suspect that now github.ref in --alias $(echo '${{ github.ref }}' | shasum | awk '{print $1}') in the deploy step below refers no longer to the branch name but to something like refs/pull/439/merge.

I haven't looked closer into how to fix this yet. Might do so tomorrow.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants