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

chore(eslint): enforce no extra semicolons #33095

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

yasuaki640
Copy link
Contributor

Issue # (if applicable)

Closes #.

Reason for this change

Enforced no extra semicolons by adding an eslint rule.

Description of changes

I would like to reduce noise during development by reducing the number of warnings output by the IDE.

Describe any new or updated permissions being added

Description of how you validated changes

Checklist


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

@yasuaki640 yasuaki640 requested a review from a team as a code owner January 23, 2025 13:21
@github-actions github-actions bot added the repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK label Jan 23, 2025
@aws-cdk-automation aws-cdk-automation requested a review from a team January 23, 2025 13:21
@github-actions github-actions bot added the p2 label Jan 23, 2025
Copy link
Collaborator

@aws-cdk-automation aws-cdk-automation left a comment

Choose a reason for hiding this comment

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

(This review is outdated)

@yasuaki640 yasuaki640 force-pushed the remove-uneccessary-semicolon branch from ef87954 to 1c3e3be Compare January 23, 2025 14:14
@mrgrain mrgrain added the pr/do-not-merge This PR should not be merged at this time. label Jan 23, 2025
@mrgrain
Copy link
Contributor

mrgrain commented Jan 23, 2025

Thanks for the contribution @yasuaki640 Really appreciate it! Unfortunately I have to block this until a few important PRs on the CLI have been merged. They are moving some files around, so this will be a bit too risky with regards to conflicts. I will keep you posted and/or do the fix up myself.

Copy link

codecov bot commented Jan 23, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.79%. Comparing base (988043e) to head (0ae503b).

Additional details and impacted files
@@           Coverage Diff           @@
##             main   #33095   +/-   ##
=======================================
  Coverage   80.79%   80.79%           
=======================================
  Files         232      232           
  Lines       14110    14110           
  Branches     2453     2453           
=======================================
  Hits        11400    11400           
  Misses       2430     2430           
  Partials      280      280           
Flag Coverage Δ
suite.unit 80.79% <ø> (ø)

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

Components Coverage Δ
packages/aws-cdk 79.53% <ø> (ø)
packages/aws-cdk-lib/core 82.17% <ø> (ø)

@mrgrain
Copy link
Contributor

mrgrain commented Jan 23, 2025

Thanks for the contribution @yasuaki640 Really appreciate it! Unfortunately I have to block this until a few important PRs on the CLI have been merged. They are moving some files around, so this will be a bit too risky with regards to conflicts. I will keep you posted and/or do the fix up myself.

Actually, @kaizencc had a better idea. We can turn off the new rule on the aws-cdk package and just don't touch it. Then your PR can be merged without problems.

@kaizencc
Copy link
Contributor

I will get this over the finish line @yasuaki640! thank you for this, it is very helpful

@kaizencc
Copy link
Contributor

@yasuaki640 I wasn't able to push to your branch, so I made this PR instead: #33101

if you would like your PR to be merged, please cherry-pick this commit c33a6829ffe8c100a3f9c34d660e04970fbe2f0b to your branch and then I'll approve!

@kaizencc
Copy link
Contributor

@yasuaki640 we're gonna get lots of merge conflicts the longer this waits. if you don't have the time, are you ok with me taking this over and adding you as a co-author?

@aws-cdk-automation aws-cdk-automation dismissed their stale review January 26, 2025 10:08

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

@yasuaki640
Copy link
Contributor Author

@kaizencc
Sorry for the late reply.
Cherry-picked commit c33a6829ffe8c100a3f9c34d660e04970fbe2f0b.
I have also given push permission to the branch to mainter, just in case.

image

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
  • Commit ID: 0ae503b
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@aws-cdk-automation aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Jan 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p2 pr/do-not-merge This PR should not be merged at this time. pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. repeat-contributor [Pilot] contributed between 3-5 PRs to the CDK
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants