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: update demo workflow to fix checkout issue #181

Merged
merged 3 commits into from
May 24, 2022

Conversation

giautm
Copy link
Contributor

@giautm giautm commented May 15, 2022

Linked issue

Fix the wrong commit issue with the solution from actions/checkout#331 (comment)

Additional context

@EvanBacon EvanBacon requested a review from byCedric May 15, 2022 19:26
Copy link
Contributor

@EvanBacon EvanBacon left a comment

Choose a reason for hiding this comment

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

lgtm, curious what @byCedric thinks

@byCedric
Copy link
Member

byCedric commented May 16, 2022

Good catch, I think I found a couple of other issues too:

  1. Looks like Github might have restricted the default github token even more. When I run this as is, I get the Error: Resource not accessible by integration error. I think this happens when the bot wants to add the 👍 reaction, but it can't "write to pulls" (see docs). Should be fixable by adding:
    permissions:
      pull-requests: write
    
  2. We aren't restricting the comments on pull requests only. Because issue_comment can be either a comment on an issue, or a comment on a pull, this causes the 1st issue but also opens the way for unexpected behavior (or unexpected action triggers, and thus wasting time) for non-pull comments. Should be fixable by adding:
    if: ${{ github.event.issue.pull_request }}
    

Then two minor things of which I'm not a super fan.

  1. Checking out the main branch first, then the pull, kind of wastes a small amount of time. Would be nice if we could checkout the PR directly, e.g.
    steps:
      - name: 🏗 Setup repo
        uses: actions/checkout@v3
        with:
          ref: refs/pull/${{ github.event.issue.number }}/merge
    
  2. For every run, you'd have to create a new comment. It's a minor thing that mostly pops up during testing, but would be nice if we could also just "trigger on edits", so we can update the comment to re-trigger it. e.g.
    on:
      issue_comment:
        types: [created, edited]
    concurrency: 
      group: expo-bot-${{ github.event.issue.number }}
      cancel-in-progress: true
    

Then, some random thoughts to make feedback a bit more clear when running from actions (this might be irrelevant as soon as we move the bot to be a github app).

  1. We could add a reaction with ⌛ in the action's pre hook. With this, we add feedback that the bot has started and is working it's way to perform the action we need.
  2. We could add the 👍 or ❌ reactions in the action's post hook whenever it failed or succeed. I think this might increase the feedback loop even more 😄

@byCedric
Copy link
Member

byCedric commented May 16, 2022

To show an example that seems to work perfectly (including the listed 2 issues and 2 minor improvements above):

name: Expo Comment Bot
on:
  # minor 2 - also trigger on updates
  issue_comment:
    types: [created, edited]
concurrency: 
  # minor 2 - limit the max concurrency to only 1 active action per pull
  group: bot-${{ github.event.issue.number }}
  cancel-in-progress: true
jobs:
  bot:
    runs-on: ubuntu-latest
    # issue 2 - only trigger from comments on pulls
    if: ${{ github.event.issue.pull_request }}
    # issue 1 - allow the bot to comment on pulls
    permissions:
      pull-requests: write
    steps:
      - name: 🏗 Setup repo
        uses: actions/checkout@v2
        with:
          # minor - 1 checkout the repo on the pull
          ref: refs/pull/${{ github.event.issue.number }}/merge
      - name: 🏗 Setup Node
        uses: actions/setup-node@v2
        with:
          node-version: 16.x
          cache: yarn
      - name: 🏗 Setup Expo
        uses: expo/expo-github-action@v7
        with:
          eas-version: latest
          expo-version: latest
          token: ${{ secrets.EXPO_TOKEN }}
      - name: 📦 Install dependencies
        run: yarn install
      - name: 🤖 Run expo command
        uses: expo/expo-github-action/command@v7

@giautm
Copy link
Contributor Author

giautm commented May 16, 2022

  1. We could add a reaction with ⌛ in the action's pre hook. With this, we add feedback that the bot has started and is working it's way to perform the action we need.
  2. We could add the 👍 or ❌ reactions in the action's post hook whenever it failed or succeed. I think this might increase the feedback loop even more 😄

I'm working on add check status for it

command/README.md Outdated Show resolved Hide resolved
Co-authored-by: Cedric van Putten <me@bycedric.com>
@giautm
Copy link
Contributor Author

giautm commented May 17, 2022

@EvanBacon, @byCedric: Can we merge this PR?

@byCedric byCedric merged commit 8e2559c into expo:main May 24, 2022
@giautm giautm deleted the gh-comments branch May 24, 2022 17:27
github-actions bot pushed a commit that referenced this pull request Jan 4, 2023
## [7.2.1](7.2.0...7.2.1) (2023-01-04)

### Code changes

* drop rimraf dependency ([#203](#203)) ([830e647](830e647))
* update actions dependencies ([#202](#202)) ([259d15d](259d15d))
* update dev dependencies ([#204](#204)) ([5a73f07](5a73f07))

### Other chores

* bump @actions/core from 1.6.0 to 1.9.1 ([#191](#191)) ([670b449](670b449))
* bump ansi-regex from 5.0.0 to 5.0.1 ([#198](#198)) ([d63621e](d63621e))
* bump json5 from 1.0.1 to 1.0.2 ([#196](#196)) ([5a8228a](5a8228a))
* bump minimatch from 3.0.4 to 3.1.2 ([#197](#197)) ([5619369](5619369))
* bump qs from 6.5.2 to 6.5.3 ([#195](#195)) ([3beea56](3beea56))
* bump semantic-release from 18.0.1 to 19.0.3 ([#185](#185)) ([079b95e](079b95e))
* bump semver-regex from 3.1.3 to 3.1.4 ([#183](#183)) ([6b82e9e](6b82e9e))
* rebuild all actions ([e6ba70c](e6ba70c))
* update workflow actions to latest ([3cecf2e](3cecf2e))

### Documentation changes

* **command:** update readme to fix badge ([#201](#201)) ([2606508](2606508))
* **preview-comment:** add missing permissions in examples ([#187](#187)) ([5848f88](5848f88))
* update badge urls in the readmes ([#199](#199)) ([8440fd9](8440fd9))
* update demo workflow to fix checkout issue ([#181](#181)) ([8e2559c](8e2559c))
* use latest action verisons in readme ([#193](#193)) ([3f0a4b5](3f0a4b5))
@github-actions
Copy link

github-actions bot commented Jan 4, 2023

🎉 This PR is included in version 7.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants