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

Lint Bazel packages with a central lint script #5790

Merged
merged 18 commits into from
Nov 2, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 19 additions & 0 deletions BAZEL_MIGRATION.md
Original file line number Diff line number Diff line change
Expand Up @@ -403,6 +403,24 @@ Update all downstream dependencies that depend on the package to point to its lo

To find downstream packages, run `grep -r --exclude=yarn.lock --exclude-dir=node_modules "link:.*your-package-name" .` in the root of the repository.

### Move linting to the repo-wide lint script
#### Add the new Bazel package to the [repo-wide tslint tsconfig](tsconfig_tslint.json):

Add the path mapping:
```json
"paths": {
...,
"@tensorflow/the-new-package": ["the-new-package/src/index.ts"],
"@tensorflow/the-new-package/dist/*": ["the-new-package/src/*"]
```

Also, remove the package from the `exclude` list.

It's a good idea to test that linting is working on the package. Create a lint error in one if its files, e.g. `const x = "Hello, world!"` (note the double quotes), and then run `yarn lint` in the root of the repository.

#### Remove the package's own TypeScript linting scripts:
Remove the `package.json` `lint` script, the `tslint.json` file, and the cloudbuild `lint` step from the package's `cloudbuild.yml` file. Remove `tslint`-related dependencies from the package's `package.json` and run `yarn` to regenerate the `yarn.lock` file.

### Update or Remove `cloudbuild.yml`
Update the `cloudbuild.yml` to remove any steps that are now built with Bazel. These will be run by the `bazel-tests` step, which runs before other packages' steps. Any Bazel rule tagged as `ci` will be tested / build in CI.

Expand Down Expand Up @@ -433,3 +451,4 @@ Before pushing to Git, run the Bazel linter by running `yarn bazel:format` and `
* Make sure the `package.json` scripts are updated and that the package.json includes `@bazel/bazelisk` as a dev dependency.
* Make sure the package has a `build-npm` script and a `publish-npm` script. These are used by the release script.
* Check the generated bundle sizes and make sure they don't include any unexpected files. Check the `_stats` files for info on this.
* Make sure the package is added to the [repo-wide tslint tsconfig](tsconfig_tslint.json) and that its original lint scripts are removed.
5 changes: 3 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,12 @@
"terser": "^5.7.0",
"ts-morph": "^11.0.3",
"ts-node": "~8.8.2",
"tslint": "~6.1.3",
"tslint": "^6.1.3",
"tslint-no-circular-imports": "~0.7.0",
"typescript": "3.5.3"
},
"scripts": {
"lint": "tslint -p tsconfig_tslint.json",
"test-packages-ci": "yarn generate-cloudbuild-for-packages && ./scripts/run-build.sh",
"generate-cloudbuild-for-packages": "./scripts/generate_cloudbuild_for_packages.js",
"test-generate-cloudbuild": "jasmine run scripts/generate_cloudbuild_test.js",
Expand All @@ -71,7 +72,7 @@
"test-release-notes": "ts-node -s ./scripts/release_notes/run_tests.ts",
"update-tfjs-lockfiles": "ts-node -s ./scripts/update-tfjs-lockfiles",
"tag-tfjs-release": "ts-node -s ./scripts/tag-tfjs-release",
"update-cloudbuild-tests": "yarn generate-cloudbuild-for-packages tfjs-node -o scripts/cloudbuild_tfjs_node_expected.yml && yarn generate-cloudbuild-for-packages tfjs-core -o scripts/cloudbuild_tfjs_core_expected.yml",
"update-cloudbuild-tests": "yarn generate-cloudbuild-for-packages tfjs-node -o scripts/cloudbuild_tfjs_node_expected.yml && yarn generate-cloudbuild-for-packages e2e -o scripts/cloudbuild_e2e_expected.yml",
"bazel:format": "find . -type f \\( -name \"*.bzl\" -or -name WORKSPACE -or -name BUILD -or -name BUILD.bazel \\) ! -path \"*/node_modules/*\" | xargs buildifier -v --warnings=attr-cfg,attr-license,attr-non-empty,attr-output-default,attr-single-file,constant-glob,ctx-actions,ctx-args,depset-iteration,depset-union,dict-concatenation,duplicated-name,filetype,git-repository,http-archive,integer-division,load,load-on-top,native-build,native-package,out-of-order-load,output-group,package-name,package-on-top,positional-args,redefined-variable,repository-name,same-origin-load,string-iteration,unsorted-dict-items,unused-variable",
"bazel:format-check": "yarn bazel:format --mode=check",
"bazel:lint": "yarn bazel:format --lint=warn",
Expand Down
271 changes: 271 additions & 0 deletions scripts/cloudbuild_e2e_expected.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,271 @@
steps:
- name: gcr.io/learnjs-174218/release
entrypoint: yarn
id: yarn-common
args:
- install
- name: gcr.io/learnjs-174218/release
dir: scripts
id: test-generate-cloudbuild
entrypoint: yarn
args:
- test-generate-cloudbuild
waitFor:
- yarn-common
- name: gcr.io/learnjs-174218/release
id: test-run-flaky
entrypoint: yarn
args:
- test-run-flaky
waitFor:
- yarn-common
- name: gcr.io/learnjs-174218/release
id: buildifier
entrypoint: yarn
args:
- buildifier-ci
waitFor:
- yarn-common
- name: gcr.io/learnjs-174218/release
id: tslint
entrypoint: yarn
args:
- lint
waitFor:
- yarn-common
- name: gcr.io/learnjs-174218/release
id: bazel-tests
entrypoint: bash
args:
- ./scripts/run_bazel_ci_tests.sh
env:
- BROWSERSTACK_USERNAME=deeplearnjs1
- NIGHTLY=$_NIGHTLY
waitFor:
- yarn-common
secretEnv:
- BROWSERSTACK_KEY
- name: gcr.io/learnjs-174218/release
dir: link-package-core
entrypoint: yarn
id: yarn-link-package-core
args:
- install
waitFor:
- bazel-tests
- name: gcr.io/learnjs-174218/release
dir: link-package
entrypoint: yarn
id: yarn-link-package
args:
- install
waitFor:
- bazel-tests
- yarn-link-package-core
- name: gcr.io/learnjs-174218/release
dir: tfjs-converter
entrypoint: yarn
id: yarn-tfjs-converter
args:
- install
waitFor:
- yarn-common
- yarn-link-package
- name: gcr.io/learnjs-174218/release
id: create-pips-tfjs-converter
entrypoint: bash
args:
- ./tfjs-converter/scripts/create_python_pips.sh
waitFor:
- yarn-common
- yarn-link-package
- name: gcr.io/learnjs-174218/release
dir: tfjs-backend-wasm
entrypoint: yarn
id: yarn-tfjs-backend-wasm
args:
- install
waitFor:
- yarn-common
- yarn-link-package
- name: gcr.io/learnjs-174218/release
dir: tfjs-backend-wasm
entrypoint: bash
id: build-tfjs-backend-wasm
args:
- ./scripts/build-ci.sh
waitFor:
- yarn-tfjs-backend-wasm
- yarn-common
- yarn-link-package
- name: gcr.io/learnjs-174218/release
dir: tfjs-backend-wasm
entrypoint: yarn
id: lint-tfjs-backend-wasm
args:
- lint
waitFor:
- yarn-tfjs-backend-wasm
- yarn-common
- yarn-link-package
- name: gcr.io/learnjs-174218/release
dir: tfjs
entrypoint: yarn
id: yarn-tfjs
args:
- install
waitFor:
- yarn-common
- yarn-link-package
- yarn-tfjs-converter
- create-pips-tfjs-converter
- name: gcr.io/learnjs-174218/release
dir: tfjs
entrypoint: yarn
id: build-tfjs
args:
- build-ci
waitFor:
- yarn-tfjs
- yarn-common
- yarn-link-package
- yarn-tfjs-converter
- create-pips-tfjs-converter
- name: gcr.io/learnjs-174218/release
dir: tfjs
entrypoint: yarn
id: lint-tfjs
args:
- lint
waitFor:
- yarn-tfjs
- yarn-common
- yarn-link-package
- yarn-tfjs-converter
- create-pips-tfjs-converter
- name: gcr.io/learnjs-174218/release
dir: tfjs-node
entrypoint: yarn
id: yarn-tfjs-node
args:
- install
waitFor:
- yarn-common
- yarn-link-package
- yarn-tfjs
- build-tfjs
- lint-tfjs
- name: gcr.io/learnjs-174218/release
dir: tfjs-node
entrypoint: yarn
id: build-addon-tfjs-node
args:
- build-addon-from-source
waitFor:
- yarn-tfjs-node
- yarn-common
- yarn-link-package
- yarn-tfjs
- build-tfjs
- lint-tfjs
- name: gcr.io/learnjs-174218/release
dir: tfjs-node
entrypoint: yarn
id: build-tfjs-node
args:
- build-ci
waitFor:
- yarn-tfjs-node
- yarn-common
- yarn-link-package
- yarn-tfjs
- build-tfjs
- lint-tfjs
- name: gcr.io/learnjs-174218/release
dir: tfjs-node
entrypoint: yarn
id: lint-tfjs-node
args:
- lint
waitFor:
- yarn-tfjs-node
- yarn-common
- yarn-link-package
- yarn-tfjs
- build-tfjs
- lint-tfjs
- name: gcr.io/learnjs-174218/release
dir: tfjs-node
entrypoint: yarn
id: ensure-cpu-gpu-packages-align-tfjs-node
args:
- ensure-cpu-gpu-packages-align
waitFor:
- yarn-common
- yarn-link-package
- yarn-tfjs
- build-tfjs
- lint-tfjs
- name: gcr.io/learnjs-174218/release
dir: e2e
entrypoint: yarn
id: yarn-e2e
args:
- install
waitFor:
- yarn-common
- yarn-link-package
- yarn-tfjs
- build-tfjs
- lint-tfjs
- yarn-tfjs-converter
- create-pips-tfjs-converter
- yarn-tfjs-node
- build-addon-tfjs-node
- build-tfjs-node
- lint-tfjs-node
- ensure-cpu-gpu-packages-align-tfjs-node
- yarn-tfjs-backend-wasm
- build-tfjs-backend-wasm
- lint-tfjs-backend-wasm
- name: gcr.io/learnjs-174218/release
dir: e2e
entrypoint: yarn
id: test-e2e
args:
- test-ci
env:
- BROWSERSTACK_USERNAME=deeplearnjs1
- NIGHTLY=$_NIGHTLY
secretEnv:
- BROWSERSTACK_KEY
waitFor:
- yarn-e2e
- yarn-common
- yarn-link-package
- yarn-tfjs
- build-tfjs
- lint-tfjs
- yarn-tfjs-converter
- create-pips-tfjs-converter
- yarn-tfjs-node
- build-addon-tfjs-node
- build-tfjs-node
- lint-tfjs-node
- ensure-cpu-gpu-packages-align-tfjs-node
- yarn-tfjs-backend-wasm
- build-tfjs-backend-wasm
- lint-tfjs-backend-wasm
secrets:
- kmsKeyName: projects/learnjs-174218/locations/global/keyRings/tfjs/cryptoKeys/enc
secretEnv:
BROWSERSTACK_KEY: >-
CiQAkwyoIW0LcnxymzotLwaH4udVTQFBEN4AEA5CA+a3+yflL2ASPQAD8BdZnGARf78MhH5T9rQqyz9HNODwVjVIj64CTkFlUCGrP1B2HX9LXHWHLmtKutEGTeFFX9XhuBzNExA=
timeout: 3600s
logsBucket: 'gs://tfjs-build-logs'
substitutions:
_NIGHTLY: ''
options:
logStreamingOption: STREAM_ON
machineType: N1_HIGHCPU_32
substitution_option: ALLOW_LOOSE
7 changes: 7 additions & 0 deletions scripts/cloudbuild_general_config.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,13 @@ steps:
args: ['buildifier-ci']
waitFor: ['yarn-common']

# Lint Bazel package typescript files.
- name: 'gcr.io/learnjs-174218/release'
id: 'tslint'
entrypoint: 'yarn'
args: ['lint']
waitFor: ['yarn-common']

# Bazel tests
# These use a remote cache and only re-run if changes occurred, so we run them
# in every build.
Expand Down
Loading