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

Add codecov coverage uploading #308

Merged
merged 2 commits into from
May 8, 2024
Merged

Add codecov coverage uploading #308

merged 2 commits into from
May 8, 2024

Conversation

filmaj
Copy link
Contributor

@filmaj filmaj commented May 8, 2024

Summary

Describe the goal of this PR. Mention any related Issue numbers.

Requirements (place an x in each [ ])

@filmaj
Copy link
Contributor Author

filmaj commented May 8, 2024

Hmm why didn't CI run my changes to the workflow file? 🤔
https://github.com/slackapi/slack-github-action/actions/runs/9008262092/job/24749902585

@filmaj filmaj requested a review from zimeg May 8, 2024 20:42
@zimeg
Copy link
Member

zimeg commented May 8, 2024

@filmaj hmm that's super strange. The workflow file changes seem to be noticed but I'd guess the pull_request_target might use the workflow on main with the changes of the PR branch being checked out.

For testing purposes, I'm wondering if changing this to pull_request would fix this? Just to test though. I think you have the magic to use repository secrets on pull request, which motivated the staging environments and custom checkouts in the first place.

Copy link
Member

@zimeg zimeg left a comment

Choose a reason for hiding this comment

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

Otherwise, you could full send the change! It looks good to me and I'm thinking running this on main will "just work".

Comment on lines +16 to +17
- run: npm ci
- run: npm run build
Copy link
Member

Choose a reason for hiding this comment

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

👏

@filmaj
Copy link
Contributor Author

filmaj commented May 8, 2024

Ahhh yeah good eye, I think you're right, as doing these kinds of changes in other repos that use pull_request and not pull_request_target and that seemed to work fine (see this for example)

@filmaj filmaj merged commit 41d5a0a into main May 8, 2024
6 checks passed
@filmaj filmaj deleted the codecov branch May 8, 2024 21:09
@zimeg
Copy link
Member

zimeg commented May 8, 2024

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.

None yet

2 participants