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

Automate GitHub releases #61

Merged
merged 6 commits into from
Dec 4, 2023
Merged

Automate GitHub releases #61

merged 6 commits into from
Dec 4, 2023

Conversation

leordev
Copy link
Member

@leordev leordev commented Oct 30, 2023

This PR automates the trigger to create a GH Release when a changeset PR is merged.

Fix #60 (see full context context).

(Also it removes the TBDocs from the release pipeline because now we are planning to release default Typedoc similar to the DWN ones).

@changeset-bot
Copy link

changeset-bot bot commented Oct 30, 2023

⚠️ No Changeset found

Latest commit: 1683ff8

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Oct 30, 2023

TBDocs Report

✅ No errors or warnings

@tbdex/protocol

  • Project entry file: packages/protocol/src/main.ts

@tbdex/http-client

  • Project entry file: packages/http-client/src/main.ts

@tbdex/http-server

  • Project entry file: packages/http-server/src/main.ts

Updated @ 2023-10-30T22:29:04.701Z - Commit: 7471f75

@@ -25,59 +16,7 @@ permissions:
id-token: write # necessary for NPM provenance

jobs:
tbdocs-publish:
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the plan with this?

Copy link
Member Author

Choose a reason for hiding this comment

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

  1. keep using the reporter for our regular PRs (it will keep us in check regarding the missing docs and breaking api changes)
  2. automatically publish the vanilla html generated by typedoc (similarly to dwn-sdk-js one)
  3. we will link from the developer.tbd.website to this one, instead of publishing it there.

This is fruit of a conversation that I had with @ALRubinger and @mistermoe.

We agreed that developers will prefer to consume the API reference in their own ecosystem instead of getting used to the TBD website theming, which would probably have limitations compared to their natural tools. Think of javadoc for java, docs.rs for rust, pkg.go.dev for go developers and so on...

We started with js which has an ecosystem that most libs have their own website docs, so it was fine, but we are stepping back to a more standard approach, so that the website does not have embedded api references only for the js libs and the rest leaves outside... (cc @angiejones)

on:
pull_request:
types:
- closed
Copy link
Contributor

Choose a reason for hiding this comment

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

interesting terminology choice by GitHub here... to me "closed" is not the same as "merged"

@KendallWeihe
Copy link
Contributor

this lgtm ✅

but I vote let's hold off until @mistermoe has a chance to look over, which may not be until next week

@leordev
Copy link
Member Author

leordev commented Nov 15, 2023

@KendallWeihe can I go ahead and merge now?

@KendallWeihe
Copy link
Contributor

KendallWeihe commented Nov 15, 2023

@leordev I think we should add some material to the README which explains the way releases work, from the point of a developer opening a PR to the point where they can go to npmjs.com and see the new version

I wrote the Changesets section, but it's slightly misleading in that it's descriptive rather than prescriptive. I like what I've written, but it's a little daunting at first glance, and we may be better off to move it somewhere different and have a simple Release section which consists of an ordered list of steps

  1. Open PR
  2. changeset-bot will automatically comment (ex: pnpm and changesets #30 (comment)) on the PR with a reminder & recommendations for semver
  3. Run pnpm changeset locally and push changes (.changet/*.md)
  4. Merge PR into main
  5. .github/workflows/create-gh-release.yml Will automatically create a GitHub Release
  6. .github/workflows/npm-publish.yml Will automatically publish to NPM

Do I have it correct?

Have we had discussions around documenting release flows? Will release flows be standard across all implementations?

leordev and others added 2 commits November 30, 2023 16:42
Co-authored-by: Kendall Weihe <kendallweihe@gmail.com>
@leordev
Copy link
Member Author

leordev commented Nov 30, 2023

@KendallWeihe

Do I have it correct?

Yes!

I think we should add some material to the README which explains the way releases work, from the point of a developer opening a PR to the point where they can go to npmjs.com and see the new version

Done! Check it out: 10a74e1#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5

Have we had discussions around documenting release flows? Will release flows be standard across all implementations?

Not yet, we do have this issue to capture that! And I agree that we should converge into a standard for all implementations. Although each language will have its own toolset.

@mistermoe @KendallWeihe about this step: Run pnpm changeset locally and push changes (.changet/*.md) -- should the developer ALWAYS do that for EVERY PR? Or only for PRs that they intend to release a new version? If the former, each merge to main will derive a new release. Is that the intent?

@KendallWeihe
Copy link
Contributor

I still didn't love my wording, specifically this bit,

Whereafter, a pnpm changeset version command must be executed.

...which makes it sound like the developer must execute this step.

I took the liberty of adjusting things a bit further 098c1e4 @leordev what'd you think? Feel free to adjust/undo/whatevs.

# we want to allow these release deployments to complete.
concurrency:
group: ${{ github.workflow }}-${{ github.ref }}
cancel-in-progress: false
Copy link
Contributor

Choose a reason for hiding this comment

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

is there a chance that a deployment can hang, in which we'd want to cancel in progress?

@@ -4,15 +4,6 @@ on:
release:
types: [created]
workflow_dispatch:
inputs:
Copy link
Contributor

Choose a reason for hiding this comment

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

could you give me some context on why this block has been removed?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since there's no more docs publishing in the release process it does not make sense to have these options anymore!

@leordev leordev merged commit 32e8b82 into main Dec 4, 2023
8 checks passed
@leordev leordev deleted the leordev/automate-gh-release branch December 4, 2023 20:37
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.

Simplify the release process
3 participants