Skip to content

Conversation

mrgrain
Copy link
Contributor

@mrgrain mrgrain commented May 7, 2025

The rules file got a bit large and hard to maintain/understand what's going on. In this change we are splitting the file up into sensible categories.

Also includes a projen upgrade to uses @stylistic for formatting rules and fixes a couple of rules to not use an array when they prefer to not use one (based on VSCode eslint plugin warnings). Places where some of the affected rules are disabled needed updating as well.

There are a bunch of small formatting changes to type statements. These didn't get picked up previously because the built-in eslint rules don't support TypeScript, but the @stylistic ones do.

The lack of any other changes to the codebase proves that these fixes are alright.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

@github-actions github-actions bot added the p2 label May 7, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team May 7, 2025 15:25
Copy link

github-actions bot commented May 7, 2025

Total lines changed 2553 is greater than 1000. Please consider breaking this PR down.

@mrgrain mrgrain added the pr/exempt-size-check Skips PR size check label May 7, 2025
@@ -99,7 +99,7 @@ export default class TestEnvironment extends NodeEnvironment implements JestEnvi
type TestDescription = PartialBy<Pick<Circus.TestEntry, 'name' | 'parent'>, 'parent'>;

// Utility type to make specific fields optional
type PartialBy<T, K extends keyof T> = Omit<T, K> & Partial<Pick<T, K>>
type PartialBy<T, K extends keyof T> = Omit<T, K> & Partial<Pick<T, K>>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't picked up before because the default eslint semi rules doesn't do TypeScript.

@@ -1,7 +1,7 @@
/**
* @deprecated
*/
export type Obj<T> = {[key: string]: T};
export type Obj<T> = { [key: string]: T };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't picked up before because the default eslint key-spacing rules doesn't do TypeScript.

@@ -25,7 +25,7 @@ export function cached<A extends object, B>(obj: A, sym: symbol, fn: () => B): B
* @deprecated
*/
export interface ContextProviderPlugin {
getValue(args: {[key: string]: any}): Promise<any>;
getValue(args: { [key: string]: any }): Promise<any>;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wasn't picked up before because the default eslint key-spacing rules doesn't do TypeScript.

@mrgrain mrgrain force-pushed the mrgrain/chore/re-org-eslint branch from cb48e59 to 5adcb38 Compare May 7, 2025 15:49
@mrgrain mrgrain temporarily deployed to integ-approval May 7, 2025 15:49 — with GitHub Actions Inactive
Signed-off-by: github-actions <[email protected]>
@codecov-commenter
Copy link

codecov-commenter commented May 7, 2025

Codecov Report

Attention: Patch coverage is 50.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 79.18%. Comparing base (89a2bcf) to head (78fd9c1).

Files with missing lines Patch % Lines
packages/aws-cdk/lib/cli/cdk-toolkit.ts 0.00% 2 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main     #466   +/-   ##
=======================================
  Coverage   79.18%   79.18%           
=======================================
  Files          54       54           
  Lines        6912     6912           
  Branches      773      773           
=======================================
  Hits         5473     5473           
  Misses       1421     1421           
  Partials       18       18           
Flag Coverage Δ
suite.unit 79.18% <50.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mrgrain mrgrain added the pr/exempt-integ-test Skips the integ test steps if set. label May 7, 2025
@mrgrain mrgrain closed this May 7, 2025
auto-merge was automatically disabled May 7, 2025 16:44

Pull request was closed

@mrgrain mrgrain reopened this May 7, 2025
@mrgrain
Copy link
Contributor Author

mrgrain commented May 7, 2025

pr/exempt-integ-test to bypass unrelated integ test failure. This PR does not require integ testing since no functional code is changed.

@mrgrain mrgrain added this pull request to the merge queue May 7, 2025
Merged via the queue into main with commit 3c10a42 May 7, 2025
41 of 43 checks passed
@mrgrain mrgrain deleted the mrgrain/chore/re-org-eslint branch May 7, 2025 17:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2 pr/exempt-integ-test Skips the integ test steps if set. pr/exempt-size-check Skips PR size check
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants