-
Notifications
You must be signed in to change notification settings - Fork 97
[No QA] Fix GitHub actions practices #733
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
Conversation
.github/workflows/cla.yml
Outdated
| regex: '\s*recheck\s*' | ||
|
|
||
| - name: CLA Assistant | ||
| if: ${{ steps.recheck.outputs.match != '' || steps.sign.outputs.match != '' || github.event_name == 'pull_request_target' }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This condition was always evaluating to true because there were characters outside of the ${{ }}
|
|
||
| # This workflow runs when code is pushed to `main` (i.e: when a pull request is merged) | ||
| on: | ||
| push: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ignored differences in indentation across files because we aren't going to enforce a yaml style (yet). But 3-space indentation was too :eye_twitch: for me to handle
|
Shit, my local branch was out of date when I created this. Merged main, should be good now |
tomekzaw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left one comment
57ca071
tomekzaw
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One more nitpick
Co-authored-by: Tomasz Zawadzki <tomekzawadzki98@gmail.com>
Details
We are getting ready to enforce:
across the Expensify org. This PR prepares us for that to be turned on without disrupting development here.
Related Issues
https://github.com/Expensify/Expensify/issues/484931 (private repo)
Manual Tests
None.
Linked PRs
None.