Setup action to create a Pull Request that will trigger publishing to npm when merged #6

joewagner · 2023-01-02T19:18:44Z

No description provided.

joewagner · 2023-01-02T23:54:22Z

.github/workflows/release-pr-manual.yml

+# The can be run manually via the github website if needed.
+# Normally the go-tableland repo should run an action that triggers
+# this to run when there is a release of the Validator.
+name: Release PR manual run


This is nearly the same workflow as release-pr.yml, but it enables using the github UI to manually create and publish a version of the validator binaries. We can potentially remove this in the long run, but for now it should be convenient to fix any failures in runs of the release pr that will be triggered automatically.

joewagner · 2023-01-02T23:55:07Z

.github/workflows/release-pr.yml

+      - name: Setup Node Environment ⬢
+        uses: actions/setup-node@v3
+        with:
+          node-version: 18


Using node 18. This is required because the build script uses web streams and fetch.

joewagner · 2023-01-02T23:55:49Z

.github/workflows/release-pr.yml

+        run: npm install
+
+      - name: Build 🔧
+        run: npm run build


The npm run build script is where the actual binaries are being downloaded unzipped and committed to git.

Why do we want to commit them, versus just including them in the build?

The idea is that we can both have a automated build release process, and still have a chance to manually build if needed.
e.g. If someone on the team wants to build a bin for there Apple M1 they can do that.

joewagner · 2023-01-03T00:06:48Z

.github/workflows/release-pr.yml

+        name: Create Release PR
+        with:
+          # TODO: I need help setting up permissions for this
+          github-token: ${{secrets.TEXTILEIO_MACHINE_ACCESS_TOKEN}}


Not sure how this secret gets added. cc: @asutula

We have it as an org wide secret. So if we have already added it and named it that, you can access it here.

joewagner · 2023-01-03T00:18:55Z

src/build.ts

+  console.log(
+    `installing validator binaries: ${platarchs
+      .map((pa) => `${pa.name}${pa.filetype}`)
+      .join(", ")} for version: ${version as string}`


Not sure exactly why, but the ts compiler is complaining unless I use version as string 😞

This is because in new versions of ts, it doesn't like having anything other than a strict string in formatted strings. So you need to explicitly define it to be string.

I tried defining the return type as a string, but I think that the cjs/ems module specific files aren't letting the ts compiler know.
ref: https://github.com/tablelandnetwork/js-validator/pull/6/files#diff-7203482ae83495c0be48d2b5a97a8e4294ed314141d5c047d9ade6347feb8f63

joewagner · 2023-01-03T00:20:02Z

src/build.ts

  console.log(`fetching: ${url}`);

  const res = await fetch(url);
  // TODO: Seems to be a bug in the fetch typings?
-  // @ts-ignore
+  // @ts-expect-error
  const downloadReadstream = Readable.fromWeb(res.body);


I found a few other reports of this, and I think that this api is so new the typings aren't figured out quite yet.

carsonfarmer

I left a few comments in here with questions and minor nits and concerns. I've still going to approve this though because I think you could merge "as is" and we'd be fine!

carsonfarmer · 2023-01-03T18:07:02Z

.github/workflows/publish.yml

@@ -23,7 +23,7 @@ jobs:
      - name: Setup Node Environment ⬢
        uses: actions/setup-node@v1
        with:
-          node-version: 16
+          node-version: 18


should we be using a version "matrix" now? it might be nice to support testing on 16-18?

Thats a good idea. If you're good with it lets scope a different piece of work build better tests and run them on all the different combos of system + node version

Awesome, we should probably just add that matrix setup to ts-template and use that across the board for tests etc.

carsonfarmer · 2023-01-03T18:07:52Z

.github/workflows/release-pr.yml

+      - name: Setup Node Environment ⬢
+        uses: actions/setup-node@v3
+        with:
+          node-version: 18


carsonfarmer · 2023-01-03T18:08:58Z

.github/workflows/release-pr.yml

+        with:
+          node-version: 18
+
+      - name: Update version in package.json


Wait, should we be using this in our other repos? This seems like it solves some problems we've had with trying to publish and update package.json based on a git tag?!

I like that idea, It's a pretty simple and it could save a step

Awesome, ok, let's add this to ts-template, and then also into js-tableland and friends. Seems like a nicer pattern than I was able to come up with!

tablelandnetwork/js-template#37

carsonfarmer · 2023-01-03T18:09:37Z

.github/workflows/release-pr.yml

+        run: npm install
+
+      - name: Build 🔧
+        run: npm run build


Why do we want to commit them, versus just including them in the build?

carsonfarmer · 2023-01-03T18:10:27Z

.github/workflows/release-pr.yml

+        name: Create Release PR
+        with:
+          # TODO: I need help setting up permissions for this
+          github-token: ${{secrets.TEXTILEIO_MACHINE_ACCESS_TOKEN}}


We have it as an org wide secret. So if we have already added it and named it that, you can access it here.

carsonfarmer · 2023-01-03T18:11:55Z

src/build.ts

+  console.log(
+    `installing validator binaries: ${platarchs
+      .map((pa) => `${pa.name}${pa.filetype}`)
+      .join(", ")} for version: ${version as string}`


This is because in new versions of ts, it doesn't like having anything other than a strict string in formatted strings. So you need to explicitly define it to be string.

src/build.ts

joewagner · 2023-01-05T00:48:20Z

Ended up making a few more changes to how this works to streamline things.

There is now only one workflow, which can be triggered via the go tableland binaries.yml workflow, or manually via the github ui.
This repo will need access to secrets.TEXTILEIO_MACHINE_ACCESS_TOKEN I believe that is setup now, but I can't tell for sure.
@asutula @carsonfarmer when you have a chance please look this over again 🙏

joewagner · 2023-01-09T17:48:02Z

cc: @carsonfarmer I made a few updates after your first review, and this needs another 👍 before merge

carsonfarmer

Updates look good @joewagner.

carsonfarmer · 2023-01-09T18:33:54Z

.github/workflows/release-pr.yml

+        with:
+          node-version: 18
+
+      - name: Update version in package.json


tablelandnetwork/js-template#37

joewagner added 9 commits December 28, 2022 09:52

Add release pr workflow file

88debef

Rename release pr workflow

936817c

Changes to release-pr to enable testing

fcb56ac

Fix release pr script

a03641e

use release version as part of branch name

e3f723d

Setup manual or triggered release workflow

b3f43cd

Finish setting up release tag creation

e1b77c1

Update secret token names

8a194fb

cleanup and better comments

fc53625

joewagner commented Jan 3, 2023

View reviewed changes

joewagner marked this pull request as ready for review January 3, 2023 00:21

joewagner requested a review from carsonfarmer as a code owner January 3, 2023 00:21

joewagner requested a review from asutula January 3, 2023 00:21

carsonfarmer previously approved these changes Jan 3, 2023

View reviewed changes

Fix fetch type casting issue

b3f71fc

joewagner dismissed carsonfarmer’s stale review via b3f71fc January 3, 2023 21:38

joewagner added 2 commits January 4, 2023 17:12

combine release workflows into one

2bf89ce

Fix org name

67fcca0

joewagner force-pushed the joe/release branch from 32f737f to 56c0f0c Compare January 5, 2023 00:35

Cleanup

bce7333

joewagner force-pushed the joe/release branch from 56c0f0c to bce7333 Compare January 5, 2023 00:38

joewagner requested a review from carsonfarmer January 5, 2023 00:48

joewagner mentioned this pull request Jan 9, 2023

add remote workflow trigger binaries.yml tablelandnetwork/go-tableland#429

Merged

carsonfarmer approved these changes Jan 9, 2023

View reviewed changes

.github/workflows/release-pr.yml

with:

node-version: 18

- name: Update version in package.json

Copy link

Member

carsonfarmer Jan 9, 2023

Choose a reason for hiding this comment

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

tablelandnetwork/js-template#37

joewagner merged commit 572aa82 into main Jan 9, 2023

joewagner deleted the joe/release branch January 9, 2023 19:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Setup action to create a Pull Request that will trigger publishing to npm when merged #6

Setup action to create a Pull Request that will trigger publishing to npm when merged #6

joewagner commented Jan 2, 2023

joewagner Jan 2, 2023

joewagner Jan 2, 2023

carsonfarmer Jan 3, 2023

joewagner Jan 2, 2023

carsonfarmer Jan 3, 2023

joewagner Jan 3, 2023 •

edited

Loading

joewagner Jan 3, 2023

carsonfarmer Jan 3, 2023

joewagner Jan 3, 2023

carsonfarmer Jan 3, 2023

joewagner Jan 3, 2023

joewagner Jan 3, 2023

carsonfarmer left a comment

carsonfarmer Jan 3, 2023

joewagner Jan 3, 2023

carsonfarmer Jan 3, 2023

carsonfarmer Jan 3, 2023

carsonfarmer Jan 3, 2023

joewagner Jan 3, 2023

carsonfarmer Jan 3, 2023

carsonfarmer Jan 9, 2023

carsonfarmer Jan 3, 2023

carsonfarmer Jan 3, 2023

carsonfarmer Jan 3, 2023

joewagner commented Jan 5, 2023

joewagner commented Jan 9, 2023

carsonfarmer left a comment

carsonfarmer Jan 9, 2023

Setup action to create a Pull Request that will trigger publishing to npm when merged #6

Setup action to create a Pull Request that will trigger publishing to npm when merged #6

Conversation

joewagner commented Jan 2, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joewagner Jan 3, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

carsonfarmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joewagner commented Jan 5, 2023

joewagner commented Jan 9, 2023

carsonfarmer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joewagner Jan 3, 2023 •

edited

Loading