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.

Sign up for GitHub

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

Jump to bottom

fix(ci): Fix GitHub workflow behavior for forks #23117

Merged
merged 6 commits into from
Apr 14, 2023
Merged

fix(ci): Fix GitHub workflow behavior for forks #23117

merged 6 commits into from
Apr 14, 2023

Conversation

jsoref
Copy link
Contributor

@jsoref jsoref commented Feb 19, 2023

SUMMARY

My forks match the behaviors of modern forks and trip on various issues...

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TESTING INSTRUCTIONS

  1. Create a new fork of this repository
  2. Enable actions for the fork
  3. Trigger workflows (e.g. by pushing to it)

ADDITIONAL INFORMATION

  • Has associated issue:
  • Required feature flags:
  • Changes UI
  • Includes DB Migration (follow approval process in SIP-59)
    • Migration is atomic, supports rollback & is backwards-compatible
    • Confirm DB migration upgrade and downgrade tested
    • Runtime estimates and downtime expectations provided
  • Introduces new feature or API
  • Removes existing feature or API

@jsoref
chore(ci): use github.token
ca07023 
It is equivalent to secrets.GITHUB_TOKEN but always available:
https://docs.github.com/en/actions/security-guides/automatic-token-authentication#about-the-github_token-secret
@jsoref
chore(ci): bump actions/checkout
d006a10
@jsoref
chore(ci): remove spurious set-url
17a1ea3
@jsoref
chore(ci): check for secrets before running
84ff4d2
@jsoref
chore(ci): request write permissions for workflows
d9bb5e8 
https://docs.github.com/en/rest/overview/permissions-required-for-github-apps?apiVersion=2022-11-28#contents
https://github.blog/changelog/2023-02-02-github-actions-updating-the-default-github_token-permissions-to-read-only/
@jsoref
chore(ci): skip bumping for forks without tags
dc1377b 
https://github.blog/changelog/2022-07-27-you-can-now-fork-a-repo-and-copy-only-the-default-branch/
https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/proposing-changes-to-your-work-with-pull-requests/creating-and-deleting-branches-within-your-repository

The default no longer includes other branches/tags
@codecov
Copy link

codecov bot commented Feb 19, 2023 •
edited
Loading

Codecov Report

Merging #23117 (5a8041a) into master (f851e99) will not change coverage.
The diff coverage is n/a.

❗ Current head 5a8041a differs from pull request most recent head dc1377b. Consider uploading reports for the commit dc1377b to get more accurate results

@@           Coverage Diff           @@
##           master   #23117   +/-   ##
=======================================
  Coverage   67.54%   67.54%           
=======================================
  Files        1881     1881           
  Lines       72396    72396           
  Branches     7882     7882           
=======================================
  Hits        48902    48902           
  Misses      21472    21472           
  Partials     2022     2022
Flag Coverage Δ
hive 52.76% <ø> (ø)
javascript 53.88% <ø> (ø)
mysql 78.37% <ø> (ø)
postgres 78.43% <ø> (ø)
presto 52.67% <ø> (ø)
python 82.27% <ø> (ø)
sqlite 76.90% <ø> (ø)
unit 52.53% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

rusackas
rusackas approved these changes Apr 12, 2023
View reviewed changes
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me, but would love a second opinion from others more intimately familiar with secret handling in actions.

nytai
nytai approved these changes Apr 14, 2023
View reviewed changes
Copy link
Member

@nytai nytai left a comment

Choose a reason for hiding this comment

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

lgtm

@rusackas rusackas merged commit 47fd732 into apache:master Apr 14, 2023
@jsoref jsoref deleted the chore-ci branch April 14, 2023 18:09
sebastianliebscher pushed a commit to sebastianliebscher/superset that referenced this pull request Apr 28, 2023
@jsoref @sebastianliebscher
fix(ci): Fix GitHub workflow behavior for forks (apache#23117)
59caf96
@sebastianliebscher sebastianliebscher mentioned this pull request Sep 9, 2023
Closed
3 tasks
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 3.0.0 labels Mar 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rusackas rusackas rusackas approved these changes

@nytai nytai nytai approved these changes

@robdiciuccio robdiciuccio Awaiting requested review from robdiciuccio

@craig-rueda craig-rueda Awaiting requested review from craig-rueda craig-rueda is a code owner

@eschutho eschutho Awaiting requested review from eschutho eschutho is a code owner

@dpgaspar dpgaspar Awaiting requested review from dpgaspar dpgaspar is a code owner

@mistercrunch mistercrunch Awaiting requested review from mistercrunch mistercrunch is a code owner

Assignees
No one assigned
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels size/L 🚢 3.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@jsoref @rusackas @nytai @mistercrunch