Code Housekeeping #1367

nathanhammond · 2022-06-10T10:36:38Z

A lot of little build housekeeping bundled into one PR.

vercel · 2022-06-10T10:36:43Z

@nathanhammond is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

nathanhammond

Lots of details about what is going on here to make it easier to follow along.

nathanhammond · 2022-06-10T10:36:59Z

.gitignore

-cli/turbo-new
-cli/turbo-new.exe


These doesn't exist anymore.

I'm fine with this, but I think @jaredpalmer uses these?

How? There are no scripts in the repository to build or generate these artifacts. What are they, and can we get the thing that generates them upstreamed?

nathanhammond · 2022-06-10T10:37:20Z

.gitignore

 cli/npm/turbo-linux-mips64le/bin
 cli/npm/turbo-linux-ppc64le/bin
+cli/npm/turbo-linux-s390x/bin


Alphabetized as a consequence of regenerating.

nathanhammond · 2022-06-10T10:37:50Z

.gitignore

-!/npm/turbo-windows-32/bin
-!/npm/turbo-windows-64/bin
-!/npm/turbo-install/bin


This is unnecessary.

nathanhammond · 2022-06-10T10:38:20Z

cli/Makefile

+GO_FILES = $(shell find . -name "*.go")
+SRC_FILES = $(shell echo $GO_FILES | grep -v "_test.go")


The full set and the reduced set are both useful to prevent invalidation.

nathanhammond · 2022-06-10T10:38:50Z

cli/Makefile


-turbo: $(SRC_FILES) go.mod
+turbo: $(SRC_FILES) go.mod go.sum


Seems like the go.sum is also important and should be considered?

nathanhammond · 2022-06-10T11:01:31Z

cli/package.json

-    "cross-env": "^7.0.3",
    "copy-template-dir": "^1.4.0",
    "esbuild": "^0.14.38",
    "esbuild-register": "^3.3.2",
    "execa": "^5.0.0",
-    "expect": "^26.6.2",
    "faker": "^5.1.0",
-    "find-up": "^5.0.0",
    "fs-extra": "^9.1.0",
    "globby": "11.1.0",
    "ngraph.generators": "^19.3.0",
-    "parse-package-name": "^0.1.0",
    "shelljs": "^0.8.4",
-    "tempy": "^1.0.0",
-    "uvu": "^0.5.3",
-    "watchlist": "^0.2.3"


These are all unused, or were removed by this PR (cross-env, watchlist)

nathanhammond · 2022-06-10T11:01:37Z

cli/scripts/bump-version.js

@@ -1,6 +1,5 @@
 #!/usr/bin/env node

-const shelljs = require("shelljs");


nathanhammond · 2022-06-10T11:02:34Z

package.json

@@ -3,9 +3,6 @@
  "version": "0.0.0",
  "private": true,
  "scripts": {
-    "format": "prettier --write",


This command doesn't actually do anything and is unused.

nathanhammond · 2022-06-10T11:02:49Z

package.json

-    "deduplicate": "npx yarn-deduplicate -s fewer yarn.lock",
-    "deduplicate:check": "npx yarn-deduplicate -s fewer yarn.lock --list --fail",


Two residual commands from the yarn days. No use anymore.

nathanhammond · 2022-06-10T11:03:22Z

package.json

@@ -21,7 +18,7 @@
      "prettier --write"
    ],
    "*.go": [
-      "sh -c \"cd cli && pnpm format\""
+      "pnpm --filter cli format"


Actually leverage pnpm workspaces features.

vercel · 2022-06-10T11:38:56Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated
turbo-site	⬜️ Ignored (Inspect)		Jun 13, 2022 at 3:33AM (UTC)

nathanhammond · 2022-06-10T12:29:48Z

@gsoltis Any idea why this test might start failing? https://github.com/vercel/turborepo/runs/6830242709?check_suite_focus=true#step:8:89

(It doesn't reproduce for me locally.)

ObliviousHarmony · 2022-06-10T18:06:44Z

@nathanhammond When I was working in the repository I noticed that turbo.json references a JSON schema file that doesn't exist, as does the documentation. The JSON schema is in schema.d.ts, is this something we should be referencing instead?

gsoltis

LGTM, I would just check w/ Jared re: turbo-new. I think I tried to remove those once before.

gsoltis · 2022-06-10T18:31:44Z

.gitignore

-cli/turbo-new
-cli/turbo-new.exe


I'm fine with this, but I think @jaredpalmer uses these?

gsoltis · 2022-06-10T18:35:25Z

cli/Makefile

+vet-go: $(GO_FILES) go.mod go.sum
+	go vet ./...


Should we get rid of this entirely? It's covered under golangci. In fact, it might be better to replace it with a command to do golangci-lint run --new-from-rev=main

gsoltis · 2022-06-10T18:40:52Z

cli/Makefile


-e2e: install
-	pnpm e2e
+e2e: install turbo


This is a good change.

gsoltis · 2022-06-10T18:44:03Z

cli/npm/turbo-install/node-platform.js

@@ -18,7 +18,7 @@ const knownWindowsPackages = {
 };

 const knownUnixlikePackages = {
-  // "android arm64 LE": "turbo-android-arm64",
+  "android arm64 LE": "turbo-android-arm64",


Beyond the scope of this change for sure, but maybe we don't need all of these platforms?

nathanhammond · 2022-06-13T03:28:26Z

@ObliviousHarmony Our base repository is required to be different from everybody else. We generate an up-to-date schema.json for our own repository here:

turborepo/docs/package.json

Line 11 in 2419e2f

    
           "schema": "ts-json-schema-generator -p ./schema.d.ts -o ./public/schema.json -t Schema --minify"

You can get that generated for you using turbo run schema.

ObliviousHarmony · 2022-06-13T03:29:38Z

That makes sense, thanks for the clarification @nathanhammond

nathanhammond requested review from jaredpalmer, gsoltis, gaspar09 and tknickman as code owners June 10, 2022 10:36

nathanhammond commented Jun 10, 2022

View reviewed changes

nathanhammond force-pushed the housekeeping branch from b864262 to c9f1089 Compare June 10, 2022 11:38

nathanhammond force-pushed the housekeeping branch from c9f1089 to 821374b Compare June 10, 2022 11:45

gsoltis added the pr: fixship A PR which is approved with notes and does not require re-review. label Jun 10, 2022

gsoltis approved these changes Jun 10, 2022

View reviewed changes

nathanhammond added 4 commits June 13, 2022 11:32

Code Housekeeping

98049a9

Switch to golangci-lint for linting.

51a4e65

These are apparently used somehow.

078ca6c

We run lint in a separate CI workflow.

64f95d6

nathanhammond force-pushed the housekeeping branch from e004366 to 64f95d6 Compare June 13, 2022 03:32

nathanhammond requested a review from gsoltis June 13, 2022 03:33

nathanhammond removed the pr: fixship A PR which is approved with notes and does not require re-review. label Jun 13, 2022

kodiakhq bot merged commit 2930841 into vercel:main Jun 13, 2022

nathanhammond deleted the housekeeping branch June 13, 2022 07:39

		GO_FILES = $(shell find . -name "*.go")
		SRC_FILES = $(shell echo $GO_FILES \| grep -v "_test.go")

		@@ -1,6 +1,5 @@
		#!/usr/bin/env node

		const shelljs = require("shelljs");

		"deduplicate": "npx yarn-deduplicate -s fewer yarn.lock",
		"deduplicate:check": "npx yarn-deduplicate -s fewer yarn.lock --list --fail",

		cli/turbo-new
		cli/turbo-new.exe

		cli/turbo-new
		cli/turbo-new.exe

Code Housekeeping #1367

Code Housekeeping #1367

Uh oh!

Conversation

nathanhammond commented Jun 10, 2022

Uh oh!

vercel bot commented Jun 10, 2022

Uh oh!

nathanhammond left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vercel bot commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nathanhammond commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ObliviousHarmony commented Jun 10, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

gsoltis left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nathanhammond commented Jun 13, 2022

Uh oh!

ObliviousHarmony commented Jun 13, 2022

Uh oh!

Uh oh!

vercel bot commented Jun 10, 2022 •

edited

Loading

nathanhammond commented Jun 10, 2022 •

edited

Loading

ObliviousHarmony commented Jun 10, 2022 •

edited

Loading