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 issue with "post_peek_screenshot" action failing when it's not supposed to #520

Merged
merged 5 commits into from
Feb 25, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions .github/workflows/peek_icons.yml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@ on:
pull_request:
types: [labeled]
jobs:
build:
peek:
# four outcomes: successful check and upload,
# unsuccessful check (fail due to user),
# fail due to system, skipped
name: Peek Icons
if: github.event.label.name == 'bot:peek'
runs-on: windows-2019
Expand All @@ -20,6 +23,18 @@ jobs:
python -m pip install --upgrade pip
pip install -r ./.github/scripts/requirements.txt

- name: Save the PR number in an artifact
shell: bash
env:
PR_NUM: ${{ github.event.number }}
run: echo $PR_NUM > pr_num.txt

- name: Upload the PR number
uses: actions/upload-artifact@v2
Copy link
Member

Choose a reason for hiding this comment

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

I think it's better to use the master version here. It's more updated than the v2 branch.

Suggested change
uses: actions/upload-artifact@v2
uses: actions/upload-artifact@master

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it might be better to stay with v2 or at least a specific tag. That way, if the upload-artifact repo is updated and breaking change occurs, we don't have to worry about it until we upgrade the action.

Copy link
Member

Choose a reason for hiding this comment

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

@Thomas-Boi is right, (almost) always stick to a specific version and do not target a master or latest branch. Targeting a latest-release is not secure in terms of code injection attacks. You might wanna take a look at #395.

with:
name: pr_num
path: ./pr_num.txt

- name: Run icomoon_peek.py
env:
PR_TITLE: ${{ github.event.pull_request.title }}
Expand All @@ -36,21 +51,9 @@ jobs:
name: screenshots
path: ./screenshots/*.png

- name: Save the pr num in an artifact
shell: bash
env:
PR_NUM: ${{ github.event.number }}
run: echo $PR_NUM > pr_num.txt

- name: Upload the pr num
uses: actions/upload-artifact@v2
with:
name: pr_num
path: ./pr_num.txt

- name: Upload geckodriver.log for debugging purposes
uses: actions/upload-artifact@v2
if: failure()
with:
name: geckodriver-log
path: ./geckodriver.log
path: ./geckodriver.log
29 changes: 18 additions & 11 deletions .github/workflows/post_peek_screenshot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,16 @@ jobs:
post_screenshots_in_comment:
name: Post the screenshot
runs-on: ubuntu-18.04
if: github.event.action == 'completed' && github.event.workflow_run.conclusion != 'skipped'
env:
# three possible values: 'skipped', 'success', 'failure'
# have to print github.event to console to see these values
# note: can't use this env variable up in the if statement above for some reason.
# I don't think it's an ordering issue cause it seems 'if' is auto evaluate first
PEEK_STATUS: ${{ github.event.workflow_run.conclusion }}
steps:
- name: Check if the trigger run worked. If not, fail the current run.
if: github.event.workflow_run.conclusion != 'success'
uses: cutenode/action-always-fail@v1.0.1
- name: Check state of last run
run: echo $PEEK_STATUS

- name: Download workflow artifact
uses: dawidd6/action-download-artifact@v2.11.0
Expand All @@ -21,14 +27,14 @@ jobs:
run_id: ${{ github.event.workflow_run.id }}

- name: Read the pr_num file
if: success()
id: pr_num_reader
uses: juliangruber/read-file-action@v1.0.0
with:
path: ./pr_num/pr_num.txt

- name: Upload screenshot of the newly made icons gotten from the artifacts
id: icons_overview_img_step
if: env.PEEK_STATUS == 'success' && success()
uses: devicons/public-upload-to-imgur@v2.2.1
with:
path: ./screenshots/new_icons.png
Expand All @@ -37,17 +43,15 @@ jobs:
- name: Upload zoomed in screenshot of the newly made icons gotten from the artifacts
id: icons_detailed_img_step
uses: devicons/public-upload-to-imgur@v2.2.1
if: success()
if: env.PEEK_STATUS == 'success' && success()
with:
path: ./screenshots/screenshot_*.png
client_id: ${{secrets.IMGUR_CLIENT_ID}}

- name: Comment on the PR about the result - Success
uses: jungwinter/comment@v1 # let us comment on a specific PR
if: success()
if: env.PEEK_STATUS == 'success' && success()
env:
OVERVIEW_IMG_MARKDOWN: ${{ fromJSON(steps.icons_overview_img_step.outputs.markdown_urls)[0] }}
DETAILED_IMGS_MARKDOWN: ${{ join(fromJSON(steps.icons_detailed_img_step.outputs.markdown_urls), '') }}
MESSAGE: |
Hi there,

Expand All @@ -71,10 +75,13 @@ jobs:
type: create
issue_number: ${{ steps.pr_num_reader.outputs.content }}
token: ${{ secrets.GITHUB_TOKEN }}
body: ${{format(env.MESSAGE, env.OVERVIEW_IMG_MARKDOWN, env.DETAILED_IMGS_MARKDOWN)}}
body: >
${{ format(env.MESSAGE,
fromJSON(steps.icons_overview_img_step.outputs.markdown_urls)[0],
join(fromJSON(steps.icons_detailed_img_step.outputs.markdown_urls), '')) }}

- name: Comment on the PR about the result - Failure
if: failure() || cancelled()
if: failure() || env.PEEK_STATUS == 'failure'
uses: jungwinter/comment@v1 # let us comment on a specific PR
env:
MESSAGE: |
Expand All @@ -88,7 +95,7 @@ jobs:
- Your icon information has been added to the `devicon.json` as seen [here](https://github.com/devicons/devicon/blob/master/CONTRIBUTING.md#updateDevicon)
- Your PR title follows the format seen [here](https://github.com/devicons/devicon/blob/master/CONTRIBUTING.md#overview)

Once everything is fixed, I will try. If I still fail (sorry!), the maintainers will investigate further.
I will retry once everything is fixed. If I still fail (sorry!) or there are other erros, the maintainers will investigate.

Best of luck,
Peek Bot :relaxed:
Expand Down