Creating wonder-stuff-ci package! #628

jlauzy · 2023-04-06T23:23:43Z

Summary:

Created a new package wonder-stuff-ci. This package will contain typescript functions that are used throughout our scripts and automations ( OLC, Bitrise, Github Actions, etc.). I found a need for this when needing common functions for interacting with mobile releases.
Ported over three functions that I need and will continue adding functions into this package as they arise.

Issue: FEI-5058

Test plan:

Wrote Jest test for the functions.

changeset-bot · 2023-04-06T23:23:46Z

🦋 Changeset detected

Latest commit: 3ef0056

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@khanacademy/wonder-stuff-ci	Patch

Not sure what this means? Click here to learn what changesets are.

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

khan-actions-bot · 2023-04-06T23:24:00Z

Gerald

Required Reviewers

@Khan/frontend-infra for changes to tsconfig-build.json, yarn.lock, .changeset/clean-houses-press.md, packages/wonder-stuff-ci/README.md, packages/wonder-stuff-ci/package.json, packages/wonder-stuff-ci/tsconfig.json, packages/wonder-stuff-ci/src/buffer-to-string.ts, packages/wonder-stuff-ci/src/compare-versions.ts, packages/wonder-stuff-ci/src/exec-async.ts, packages/wonder-stuff-ci/src/extract-mobile-release-info-from-branch-name.ts, packages/wonder-stuff-ci/src/get-mobile-release-tags.ts, packages/wonder-stuff-ci/src/get-tags-from-git.ts, packages/wonder-stuff-ci/src/index.ts, packages/wonder-stuff-ci/src/types.ts, packages/wonder-stuff-ci/src/__tests__/buffer-to-string.test.ts, packages/wonder-stuff-ci/src/__tests__/compare-versions.test.ts, packages/wonder-stuff-ci/src/__tests__/extract-mobile-release-info-from-branch-name.test.ts, packages/wonder-stuff-ci/src/__tests__/get-mobile-release-tags.test.ts, packages/wonder-stuff-ci/src/__tests__/get-tags-from-git.test.ts

Don't want to be involved in this pull request? Comment #removeme and we won't notify you of further changes.

jlauzy · 2023-04-06T23:27:13Z

packages/wonder-stuff-ci/CHANGELOG.md

+
+## 0.0.1
+
+- Initial commit of porting over OLC functions for mobile release management


How should I format the change log in wonderstuff? Is there a getting started guide on creating a package in wonder-stuff? All I did was copy what another package was doing.

Ahh no - so we're using Changesets! The CHANGELOG is generated automatically, so you shouldn't make it. You can see how to do this here: https://khanacademy.atlassian.net/wiki/spaces/WB/pages/2217640195/Versioning+and+releasing+Wonder+Blocks+packages (it's Wonder Blocks but we follow the same process for Wonder Stuff)

jlauzy · 2023-04-06T23:27:55Z

packages/wonder-stuff-ci/package.json

+        "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'"
+    },
+    "devDependencies": {
+        "@types/node": "^18.15.11",


I needed to add this to resolve some types in the functions i need.

jlauzy · 2023-04-06T23:28:19Z

packages/wonder-stuff-ci/package.json

+    },
+    "devDependencies": {
+        "@types/node": "^18.15.11",
+        "ws-dev-build-settings": "^1.1.0"


What is the ws-dev-build-settings? Should I be adding the @types/node into this?

This is internal to wonder-stuff itself, these are the shared tools that we use to help with test running, builds, etc. I'm not sure if it makes sense to have the node types, or not! I'd say "no" only because not everything in wonder-stuff is designed to run in Node.

jlauzy · 2023-04-06T23:28:59Z

packages/wonder-stuff-ci/package.json

+    "version": "0.0.1",
+    "description": "Functions for creating automation and scripts.",
+    "scripts": {
+        "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'"


I was trying to get the test to run with yarn jest, but that didn't work because of typing. But using the yarn command it did work!

Ah yeah - I think this handles some of the trickiness with being in a monorepo, if I remember correctly.

Typically, you have to run yarn jest and yarn test from the root directory. This confusing command at the package level basically does that for you, so you can do yarn test in one of the package folders, and it will do the right thing.

jlauzy · 2023-04-06T23:30:19Z

tsconfig-build.json

@@ -12,5 +12,6 @@
    {"path": "./packages/wonder-stuff-sentry"},
    {"path": "./packages/wonder-stuff-server"},
    {"path": "./packages/wonder-stuff-testing"},
+    {"path": "./packages/wonder-stuff-ci"},


I was curious how the tsconfig-build.json is used in wonderstuff?

What else is needed for me to get this new package published to npm?

All of it is managed by Changesets - once you changes are approved and landed then a Changesets PR will be generated, once that's approved and landed it'll publish to NPM!

codecov · 2023-04-06T23:30:21Z

Codecov Report

Merging #628 (3ef0056) into main (02bcad1) will not change coverage.
The diff coverage is 100.00%.

Additional details and impacted files

@@            Coverage Diff            @@
##              main      #628   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           87        93    +6     
  Lines         1322      1349   +27     
  Branches       331       328    -3     
=========================================
+ Hits          1322      1349   +27

Impacted Files	Coverage Δ
packages/wonder-stuff-ci/src/buffer-to-string.ts	`100.00% <100.00%> (ø)`
packages/wonder-stuff-ci/src/compare-versions.ts	`100.00% <100.00%> (ø)`
packages/wonder-stuff-ci/src/exec-async.ts	`100.00% <100.00%> (ø)`
...rc/extract-mobile-release-info-from-branch-name.ts	`100.00% <100.00%> (ø)`
...ges/wonder-stuff-ci/src/get-mobile-release-tags.ts	`100.00% <100.00%> (ø)`
packages/wonder-stuff-ci/src/get-tags-from-git.ts	`100.00% <100.00%> (ø)`

... and 3 files with indirect coverage changes

Continue to review full report in Codecov by Sentry.

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

jlauzy · 2023-04-06T23:30:40Z

@somewhatabstract @kevinbarabash , uh did i do this right lol

Curious about what other steps I need to do to get this package publish, so that I can import it from a script I have in bitrise.

jeresig

Exciting! Suggestions and corrections inline.

jeresig · 2023-04-07T13:45:39Z

packages/wonder-stuff-ci/CHANGELOG.md

+
+## 0.0.1
+
+- Initial commit of porting over OLC functions for mobile release management


Ahh no - so we're using Changesets! The CHANGELOG is generated automatically, so you shouldn't make it. You can see how to do this here: https://khanacademy.atlassian.net/wiki/spaces/WB/pages/2217640195/Versioning+and+releasing+Wonder+Blocks+packages (it's Wonder Blocks but we follow the same process for Wonder Stuff)

jeresig · 2023-04-07T13:47:32Z

packages/wonder-stuff-ci/package.json

+    },
+    "devDependencies": {
+        "@types/node": "^18.15.11",
+        "ws-dev-build-settings": "^1.1.0"


This is internal to wonder-stuff itself, these are the shared tools that we use to help with test running, builds, etc. I'm not sure if it makes sense to have the node types, or not! I'd say "no" only because not everything in wonder-stuff is designed to run in Node.

jeresig · 2023-04-07T13:48:35Z

packages/wonder-stuff-ci/package.json

+    "version": "0.0.1",
+    "description": "Functions for creating automation and scripts.",
+    "scripts": {
+        "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'"


Ah yeah - I think this handles some of the trickiness with being in a monorepo, if I remember correctly.

jeresig · 2023-04-07T13:51:45Z

packages/wonder-stuff-ci/src/release-utils.ts

@@ -0,0 +1,70 @@
+import execProm from "./exec-prom";


I would rename this file to mobile-release-utils.ts. That being said, this logic is rather specific to our release processes!

jeresig · 2023-04-07T13:52:35Z

tsconfig-build.json

@@ -12,5 +12,6 @@
    {"path": "./packages/wonder-stuff-sentry"},
    {"path": "./packages/wonder-stuff-server"},
    {"path": "./packages/wonder-stuff-testing"},
+    {"path": "./packages/wonder-stuff-ci"},


All of it is managed by Changesets - once you changes are approved and landed then a Changesets PR will be generated, once that's approved and landed it'll publish to NPM!

jeresig · 2023-04-07T13:54:17Z

packages/wonder-stuff-ci/package.json

+    "name": "@khanacademy/wonder-stuff-ci",
+    "version": "0.0.1",
+    "description": "Functions for creating automation and scripts.",
+    "scripts": {


I think you're missing a critical index.ts file with all the things that you've created exported. Looking at wonder-stuff-core you'll probably want to add these lines here:

"module": "dist/es/index.js", "main": "dist/index.js", "types": "dist/index.d.ts",

And then make a src/index.ts file that imports your files and re-exports them again.

jeresig · 2023-04-07T13:55:21Z

packages/wonder-stuff-ci/src/release-utils.ts

+ * @param arg
+ * @returns match, if found, null otherwise
+ */
+export const isGetReleaseBranch = (arg: string) => {


I would rename this to: extractMobileReleaseBranch.

jeresig · 2023-04-07T13:55:39Z

packages/wonder-stuff-ci/src/release-utils.ts

+ *
+ * @returns all release tags sorted creation time ascending
+ */
+const allReleaseTags = async (): Promise<Array<string>> => {


I would re-name this to: allMobileReleaseTags

github-actions · 2023-04-07T18:28:20Z

Size Change: 0 B

Total Size: 4.42 kB

ℹ️ View Unchanged

Filename	Size
`packages/wonder-stuff-core/dist/browser/es/index.js`	1.78 kB
`packages/wonder-stuff-sentry/dist/browser/es/index.js`	1.66 kB
`packages/wonder-stuff-testing/dist/browser/es/index.js`	970 B

_{compressed-size-action}

kevinbarabash

LGTM. A few nits, but nothing blocking. Thanks for creating this new shared package.

kevinbarabash · 2023-04-07T18:30:16Z

packages/wonder-stuff-ci/src/exec-prom.ts

+                          stderr: bufferToString(stderr),
+                      })
+                : res({
+                      err: null,


suggestion (non-blocking): You could use null here which would allow the type for err to be Error | undefined.

kevinbarabash · 2023-04-07T18:31:12Z

packages/wonder-stuff-ci/src/exec-prom.ts

+    if (typeof input === "string") {
+        return input;
+    } else {
+        return input.toString("utf8");
+    }


suggestion (non-blocking): The following would be a bit more succinct:

Suggested change

if (typeof input === "string") {

return input;

} else {

return input.toString("utf8");

}

return (typeof input === "string") ? input : input.toString("utf8");

kevinbarabash · 2023-04-07T18:33:40Z

packages/wonder-stuff-ci/src/mobile-release-utils.ts

+    const v1v = v1.includes("-") ? v1.split("-")[1] : v1;
+    const v2v = v2.includes("-") ? v2.split("-")[1] : v2;


question: why are we checking for "-" here? The docstring says the version should be of the form <num>.<num>.<num>.

Ah, I need to update the docstring! tags are in the form unified-7.9.0, but this function also takes in raw version string 7.9.0

kevinbarabash · 2023-04-07T18:39:35Z

packages/wonder-stuff-ci/src/mobile-release-utils.ts

+    const match = arg.match(
+        /^(release\/(ios|android|unified)\/)?v?(\d+\.\d+\.\d+(-\w*)*)$/i,
+    );


praise: I like that we're using a regex for this. Maybe we can do the same for compareVersions so that there's more consistency with how we're parsing version strings. This would allow the code in that function to be less defensive (we could assume there's always three numbers in a version string).

Ah I see what you are saying

const v1p = v1v.replace(/^v/g, "").split("."); const v2p = v2v.replace(/^v/g, "").split("."); for (let i = 0; i < v1p.length || i < v2p.length; i++) { const p1 = +v1p[i] || 0; const p2 = +v2p[i] || 0; if (+p1 !== +p2) { return p1 > p2 ? 1 : -1; } }

This does not check if there are 3 numbers in the version string only. It just checks each digit that is split by "."

kevinbarabash · 2023-04-07T18:47:47Z

packages/wonder-stuff-ci/src/__tests__/mobile-release-utils.test.ts

+        "release/unified/7.8.0",
+        "release/android/7.8.0",
+        "release/ios/7.8.0",
+    ])("get the version from the release branch", (testCase: any) => {


question: does TS complain if we type the testCase as string instead of any?

hmm interestingly my IDE does not complain. I ran tsc no complaints. I even ran yarn test and no complaints.

jeresig

Thanks for the changes!

somewhatabstract

This is looking good. I have a few changes I'd like to see before this gets landed. See my inline comments for info.

somewhatabstract · 2023-04-10T17:20:01Z

packages/wonder-stuff-ci/README.md

@@ -0,0 +1,3 @@
+# wonder-stuff-ci
+
+This Wonder Stuff package contains functions for creating automation and scripts.


nit:

Suggested change

This Wonder Stuff package contains functions for creating automation and scripts.

This Wonder Stuff package contains functions for automation and scripts.

somewhatabstract · 2023-04-10T17:22:39Z

packages/wonder-stuff-ci/package.json

+    "version": "0.0.1",
+    "description": "Functions for creating automation and scripts.",
+    "scripts": {
+        "test": "bash -c 'yarn --silent --cwd \"../..\" test ${@:0} $($([[ ${@: -1} = -* ]] || [[ ${@: -1} = bash ]]) && echo $PWD)'"


Typically, you have to run yarn jest and yarn test from the root directory. This confusing command at the package level basically does that for you, so you can do yarn test in one of the package folders, and it will do the right thing.

packages/wonder-stuff-ci/src/mobile-release-utils.ts

somewhatabstract · 2023-04-10T17:30:19Z

packages/wonder-stuff-ci/src/mobile-release-utils.ts

+ *
+ * @returns all release tags sorted creation time ascending


suggestion: Don't forget a first docline as well as types on the return.

Suggested change

*

* @returns all release tags sorted creation time ascending

* Get mobile release tags.

*

* Tags are filtered to only include those matching our tag version format

* (`<tag>-<num>.<num>.<num>`), then they are sorted by the version information from

* earliest version to most recent.

*

* @returns {Promise<Array<string>>} all release tags sorted creation time ascending

packages/wonder-stuff-ci/src/mobile-release-utils.ts

packages/wonder-stuff-ci/src/__tests__/mobile-release-utils.test.ts

packages/wonder-stuff-ci/src/exec-prom.ts

packages/wonder-stuff-ci/src/mobile-release-utils.ts

jlauzy · 2023-04-10T21:47:13Z

@somewhatabstract Thanks for the feedback jeff, updated the pr based on your comments. Let me know if I missed anything!

somewhatabstract

Thanks for all these changes. This is looking great. I have one more change to suggest, but not blocking. See the inline commentary for info.

somewhatabstract · 2023-04-11T20:47:01Z

packages/wonder-stuff-ci/package.json

@@ -7,7 +7,7 @@
    },
    "name": "@khanacademy/wonder-stuff-ci",
    "version": "0.0.1",
-    "description": "Functions for creating automation and scripts.",
+    "description": "Functions for automation and scripts.",


praise: Thanks!

somewhatabstract · 2023-04-11T20:52:05Z

packages/wonder-stuff-ci/src/extract-mobile-release-info-from-branch-name.ts

+ * @param {string} The release branch of the form `[release/[unified|ios|android]]/[v]<num>.<num>.<num>[-extra]`.
+ * @returns {Object} The release version and prefix, if found; otherwise, `null`.
+ */
+export const extractMobileReleaseInfoFromBranchName = (arg: string | null) => {


suggestion: Currently, the return type is inferred from the code and as such, there isn't a concrete way to refer to this type. It would be helpful for calling code if there was one as it can be used by them to type their own arguments and variables.

Something like:

type MobileReleaseInfo = { prefix: "release/unified/" | "release/ios" | "release/android", version: string, };

And then:

Suggested change

export const extractMobileReleaseInfoFromBranchName = (arg: string | null) => {

export const extractMobileReleaseInfoFromBranchName =

(arg: string | null): MobileReleaseInfo => {

somewhatabstract · 2023-04-11T20:54:47Z

packages/wonder-stuff-ci/src/extract-mobile-release-info-from-branch-name.ts

+ * }
+ *
+ * @param {string} The release branch of the form `[release/[unified|ios|android]]/[v]<num>.<num>.<num>[-extra]`.
+ * @returns {Object} The release version and prefix, if found; otherwise, `null`.


suggestion: If you create the type as suggested below and then replace {Object} with {MobileReleaseInfo}, which gives folks a little nicer info in their IDEs.

somewhatabstract · 2023-04-12T18:59:50Z

packages/wonder-stuff-ci/src/types.ts

+/**
+ * The information about a release branch.
+ */
+type MobileReleaseBranchInfo = {


suggestion: Don't forget to export this type from index.ts so consumers of the API can use it.

export type {MobileReleaseBranchInfo} from "./types";

or

export * from "./types";

somewhatabstract · 2023-04-12T19:02:10Z

packages/wonder-stuff-ci/src/extract-mobile-release-info-from-branch-name.ts

+ * }
+ *
+ * @param {string} The release branch of the form `[release/[unified|ios|android]]/[v]<num>.<num>.<num>[-extra]`.
+ * @returns {MobileReleaseInfo} The release version and prefix, if found; otherwise, `null`.


suggestion:

Suggested change

* @returns {MobileReleaseInfo} The release version and prefix, if found; otherwise, `null`.

* @returns {MobileReleasBranchInfo} The release version and prefix, if found; otherwise, `null`.

Adding three functions that I need for CI scripts that are used in OLC

8fd79c5

jlauzy self-assigned this Apr 6, 2023

khan-actions-bot requested a review from a team April 6, 2023 23:23

jlauzy changed the title ~~Adding three functions that I need for CI scripts that are used in OLC~~ Creating wonder-stuff-ci package! Apr 6, 2023

jlauzy commented Apr 6, 2023

View reviewed changes

jeresig requested changes Apr 7, 2023

View reviewed changes

Jeffrey Lau added 2 commits April 7, 2023 11:01

updated the PR based on comments

b4a779e

adding changeset record

ccc5a16

kevinbarabash approved these changes Apr 7, 2023

View reviewed changes

Jeffrey Lau added 2 commits April 7, 2023 14:50

added more unit tests

afb8d7d

updated based on pr comments

c9a0fed

jlauzy requested a review from jeresig April 7, 2023 22:00

jeresig approved these changes Apr 10, 2023

View reviewed changes

get 100% coverage

062cbe3

somewhatabstract requested changes Apr 10, 2023

View reviewed changes

updated based on pr comments from jeff

637aa46

jlauzy requested a review from somewhatabstract April 10, 2023 21:46

updated readme

74c1788

somewhatabstract approved these changes Apr 11, 2023

View reviewed changes

added a type for mobile release info

3ef0056

somewhatabstract reviewed Apr 12, 2023

View reviewed changes

jlauzy merged commit d2fbdcc into main Apr 12, 2023

jlauzy deleted the FEI-5058-CreateWonderStuffCI branch April 12, 2023 22:13

jlauzy mentioned this pull request Apr 12, 2023

Added a logger for sending logs to GCP #636

Merged


		## 0.0.1

		- Initial commit of porting over OLC functions for mobile release management

		const v1v = v1.includes("-") ? v1.split("-")[1] : v1;
		const v2v = v2.includes("-") ? v2.split("-")[1] : v2;

		@@ -0,0 +1,3 @@
		# wonder-stuff-ci

		This Wonder Stuff package contains functions for creating automation and scripts.

- *
- * @returns all release tags sorted creation time ascending
+ * Get mobile release tags.
+ *
+ * Tags are filtered to only include those matching our tag version format
+ * (`<tag>-<num>.<num>.<num>`), then they are sorted by the version information from
+ * earliest version to most recent.
+ *
+ * @returns {Promise<Array<string>>} all release tags sorted creation time ascending

	export const extractMobileReleaseInfoFromBranchName = (arg: string \| null) => {
	export const extractMobileReleaseInfoFromBranchName =
	(arg: string \| null): MobileReleaseInfo => {

	* @returns {MobileReleaseInfo} The release version and prefix, if found; otherwise, `null`.
	* @returns {MobileReleasBranchInfo} The release version and prefix, if found; otherwise, `null`.

Creating wonder-stuff-ci package! #628

Creating wonder-stuff-ci package! #628

Conversation

jlauzy commented Apr 6, 2023 • edited Loading

Summary:

Test plan:

changeset-bot bot commented Apr 6, 2023 • edited Loading

🦋 Changeset detected

khan-actions-bot commented Apr 6, 2023 • edited Loading

Gerald

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

codecov bot commented Apr 6, 2023 • edited Loading

Codecov Report

jlauzy commented Apr 6, 2023 • edited Loading

jeresig 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

github-actions bot commented Apr 7, 2023 • edited Loading

kevinbarabash 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

jeresig left a comment

Choose a reason for hiding this comment

somewhatabstract 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

jlauzy commented Apr 10, 2023

somewhatabstract 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

jlauzy commented Apr 6, 2023 •

edited

Loading

changeset-bot bot commented Apr 6, 2023 •

edited

Loading

khan-actions-bot commented Apr 6, 2023 •

edited

Loading

codecov bot commented Apr 6, 2023 •

edited

Loading

jlauzy commented Apr 6, 2023 •

edited

Loading

github-actions bot commented Apr 7, 2023 •

edited

Loading