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

Auto update flow-go github action #981

Merged
merged 2 commits into from
Jun 7, 2021
Merged

Conversation

janezpodhostnik
Copy link
Contributor

@janezpodhostnik janezpodhostnik commented Jun 4, 2021

Closes https://github.com/dapperlabs/flow-internal/issues/1517

Description

A github action that creates a PR in flow-go to update cadence version every time a PR is merged into master in cadence. The PRs in flow-go are based on the master branch or the latest auto-cadence-upgrade/* branch if it exists

I tested this on private repos, but it might still need some tweaking on the go get && make tidy part.


  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

@codecov-commenter
Copy link

codecov-commenter commented Jun 4, 2021

Codecov Report

Merging #981 (7732b0d) into master (7797f9f) will not change coverage.
The diff coverage is n/a.

❗ Current head 7732b0d differs from pull request most recent head 6bfdd75. Consider uploading reports for the commit 6bfdd75 to get more accurate results
Impacted file tree graph

@@           Coverage Diff           @@
##           master     #981   +/-   ##
=======================================
  Coverage   75.46%   75.46%           
=======================================
  Files         268      268           
  Lines       32870    32870           
=======================================
  Hits        24805    24805           
  Misses       6929     6929           
  Partials     1136     1136           
Flag Coverage Δ
unittests 75.46% <ø> (ø)

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


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7797f9f...6bfdd75. Read the comment docs.

Copy link
Member

@turbolent turbolent left a comment

Choose a reason for hiding this comment

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

Great work! Looking forward to using this 👍

Comment on lines +7 to +16
pull_request:
branches:
- master
types: [closed]

jobs:
sync-flow-go:
runs-on: ubuntu-latest
# the PR could have been closed otherwise. Only run if it has actually ben merged
if: github.event.pull_request.merged == true
Copy link
Member

Choose a reason for hiding this comment

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

Maybe this can be simplified to just run on each push to master?

But then I guess the events at the bottom (github.event.pull_request) aren't available / would need to change?

Suggested change
pull_request:
branches:
- master
types: [closed]
jobs:
sync-flow-go:
runs-on: ubuntu-latest
# the PR could have been closed otherwise. Only run if it has actually ben merged
if: github.event.pull_request.merged == true
push:
branches:
- master
jobs:
sync-flow-go:
runs-on: ubuntu-latest

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The convenience of having a reference of a Cadence PR in the generated flow-go PR would be lost... Not sure that would be the best approach.

Are there pushes to master that happen without PRs?

.github/workflows/sync-flow-go.yml Outdated Show resolved Hide resolved
Co-authored-by: Bastian Müller <bastian@axiomzen.co>
@janezpodhostnik
Copy link
Contributor Author

@turbolent when you have some time, could you add the secrets.REMOTE_REPO_PAT (or we could rename it to FLOW_GO_PAT) to the repo secrets?

@turbolent
Copy link
Member

Added the secret, should be good to go now!

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

Successfully merging this pull request may close these issues.

None yet

3 participants