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

Implement package linter #148496

Merged
merged 30 commits into from
Jan 9, 2023
Merged

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Jan 6, 2023

This PR implements a linter like the TS Project linter, except for packages in the repo. It does this by extracting the reusable bits from the TS Project linter and reusing them for the project linter. The only rule that exists for packages right now is that the "name" in the package.json file matches the "id" in Kibana.jsonc. The goal is to use a rule to migrate kibana.json files on the future.

Additionally, a new rule for validating the indentation of tsconfig.json files was added.

Validating and fixing violations is what has triggered review by so many teams, but we plan to treat those review requests as notifications of the changes and not as blockers for merging.

@spalger spalger added Team:Operations Team label for Operations Team release_note:skip Skip the PR/issue when compiling release notes labels Jan 7, 2023
@spalger spalger requested review from a team as code owners January 7, 2023 00:04
@botelastic botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Jan 7, 2023
@elasticmachine
Copy link
Contributor

Pinging @elastic/uptime (Team:uptime)

Copy link
Contributor

@TinaHeiligers TinaHeiligers left a comment

Choose a reason for hiding this comment

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

Changes in core-owned packages are white-space related.
LGTM

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

Visualizations team changes LGTM!

Copy link
Contributor

@PhilippeOberti PhilippeOberti left a comment

Choose a reason for hiding this comment

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

Protections Experience changes LGTM!

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

ML team changes LGTM

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Presentation team changes in canvas tsconfig LGTM. Does the new linting system highlight unused references in the tsconfig now?

@spalger
Copy link
Contributor Author

spalger commented Jan 9, 2023

Presentation team changes in canvas tsconfig LGTM. Does the new linting system highlight unused references in the tsconfig now?

@ThomThomson It doesn't just highlight them, it automatically removes then on CI and when you execute node scripts/lint_ts_projects --fix

@spalger spalger requested a review from a team as a code owner January 9, 2023 21:51
Copy link
Member

@mistic mistic left a comment

Choose a reason for hiding this comment

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

lgtm

@kibana-ci
Copy link
Collaborator

kibana-ci commented Jan 9, 2023

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Public APIs missing comments

Total count of every public API that lacks a comment. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats comments for more detailed information.

id before after diff
@kbn/json-ast - 28 +28
@kbn/repo-file-maps - 18 +18
@kbn/repo-linter - 38 +38
@kbn/sort-package-json 2 0 -2
@kbn/ts-project-linter 9 - -9
@kbn/ts-projects 16 25 +9
total +82

Any counts in public APIs

Total count of every any typed public API. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats any for more detailed information.

id before after diff
@kbn/json-ast - 2 +2

Public APIs missing exports

Total count of every type that is part of your API that should be exported but is not. This will cause broken links in the API documentation system. Target amount is 0. Run node scripts/build_api_docs --plugin [yourplugin] --stats exports for more detailed information.

id before after diff
@kbn/bazel-packages 1 - -1
@kbn/package-map 1 - -1
@kbn/repo-linter - 5 +5
@kbn/repo-packages - 3 +3
@kbn/ts-project-linter 1 - -1
total +5
Unknown metric groups

API count

id before after diff
@kbn/json-ast - 32 +32
@kbn/repo-file-maps - 18 +18
@kbn/repo-linter - 40 +40
@kbn/repo-path 11 12 +1
@kbn/sort-package-json 2 0 -2
@kbn/ts-project-linter 9 - -9
@kbn/ts-projects 27 38 +11
total +91

ESLint disabled in files

id before after diff
@kbn/bazel-packages 1 - -1
@kbn/import-locator - 2 +2
@kbn/repo-packages - 1 +1
@kbn/ts-project-linter 2 - -2
total -0

Total ESLint disabled count

id before after diff
@kbn/bazel-packages 1 - -1
@kbn/import-locator - 2 +2
@kbn/repo-packages - 1 +1
@kbn/ts-project-linter 2 - -2
total -0

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

@spalger spalger merged commit d6be4a4 into elastic:main Jan 9, 2023
@spalger spalger deleted the implement/package-linter branch January 9, 2023 23:49
@kibanamachine kibanamachine added v8.7.0 backport:skip This commit does not require backporting labels Jan 9, 2023
jennypavlova pushed a commit to jennypavlova/kibana that referenced this pull request Jan 13, 2023
This PR implements a linter like the TS Project linter, except for
packages in the repo. It does this by extracting the reusable bits from
the TS Project linter and reusing them for the project linter. The only
rule that exists for packages right now is that the "name" in the
package.json file matches the "id" in Kibana.jsonc. The goal is to use a
rule to migrate kibana.json files on the future.

Additionally, a new rule for validating the indentation of tsconfig.json
files was added.

Validating and fixing violations is what has triggered review by so many
teams, but we plan to treat those review requests as notifications of
the changes and not as blockers for merging.

Co-authored-by: kibanamachine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting release_note:skip Skip the PR/issue when compiling release notes Team:Operations Team label for Operations Team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v8.7.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.