-
Notifications
You must be signed in to change notification settings - Fork 518
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 automated release workflows #366
Conversation
Update typescript and gts to new major versions. Additionally update eslint and plugins to latest versions and fix some other minor issues like repository field in package.json.
Codecov Report
@@ Coverage Diff @@
## main #366 +/- ##
=======================================
Coverage 94.43% 94.43%
=======================================
Files 11 11
Lines 431 431
Branches 48 48
=======================================
Hits 407 407
Misses 24 24 |
Darn, well I'm no good at video editing so it's too big to embed here on GitHub, but you can download the recording of the flow here: https://demo-video-otel-js-9y182zqovhunxlwv.s3-us-west-2.amazonaws.com/Screen+Recording+2021-02-25+at+8.45.39+PM.mov Also, I'll be out next week. Unfortunate timing, but I'll likely be unresponsive til around 3/7. |
@dyladan can you all take a look when you get a chance? I'm back from my vacation and will be able to address any comments. |
@dyladan any chance for a review? |
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.
overall LGTM, though i have a question about permission: who can launch the release workflow from github ? People with write access or do it need admin permission on the repo ?
@vmarchaud Anyone with write access can run it, but the manually-runnable workflow only creates the release PR. The PR still needs to be approved before merging, which actually triggers the publishing. So someone with write access could spam PRs (and hopefully get their write access revoked) but not actually release anything to the wild. |
Apologize for late review. I was on vacation all last week |
@obecny since this is fairly important I think we should have approval from all maintainers before merging |
Which problem is this PR solving?
Short description of the changes