Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

feat(rule-tester): stricter rule test validations#9086

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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation

Vinccool96
Copy link
Contributor

PR Checklist

Overview

Brings in changes fromeslint/eslint#17654. This is pretty much a copy & paste.

@typescript-eslint
Copy link
Contributor

Thanks for the PR,@Vinccool96!

typescript-eslint is a 100% community driven project, and we are incredibly grateful that you are contributing to that community.

The core maintainers work on this in their personal time, so please understand that it may not be possible for them to review your work immediately.

Thanks again!

🙏Please, if you or your company is finding typescript-eslint valuable, help us sustain the project by sponsoring it transparently onhttps://opencollective.com/typescript-eslint.

@netlifyNetlify
Copy link

netlifybot commentedMay 13, 2024•
edited
Loading

✅ Deploy Preview fortypescript-eslint ready!

NameLink
🔨 Latest commit3bdc61e
🔍 Latest deploy loghttps://app.netlify.com/sites/typescript-eslint/deploys/6686389c7fa35000084015a2
😎 Deploy Previewhttps://deploy-preview-9086--typescript-eslint.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		1 paths audited
Performance: 98 (🟢 up 4 from production)
Accessibility: 100 (no change from production)
Best Practices: 92 (no change from production)
SEO: 90 (no change from production)
PWA: 80 (no change from production)
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to yourNetlify site configuration.

@Vinccool96Vinccool96 changed the titlefeat(rule-tester): Stricter rule test validationsfeat(rule-tester): stricter rule test validationsMay 13, 2024
@nx-cloudNx Cloud
Copy link

nx-cloudbot commentedMay 13, 2024•
edited
Loading

☁️ Nx Cloud Report

CI is running/has finished running commands for commit3bdc61e. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution

✅ Successfully ran 1 target

Sent with 💌 fromNxCloud.

@auvredauvred added the awaiting responseIssues waiting for a reply from the OP or another party labelMay 13, 2024
@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch fromaca9d06 tobde1e6eCompareMay 13, 2024 15:19
@Vinccool96
Copy link
ContributorAuthor

Vinccool96 commentedMay 13, 2024•
edited
Loading

I'm gonna need some help for the failing tests

Never mind, there was way less things to fix than I thought.

@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch from901db33 to6e71280CompareMay 13, 2024 20:15
@Vinccool96Vinccool96 requested a review fromauvredMay 13, 2024 21:19
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelMay 13, 2024
@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch 2 times, most recently from06d3edf to5f55140CompareMay 26, 2024 17:09
@Vinccool96
Copy link
ContributorAuthor

@auvred Is everything good to merge?

@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch from5f55140 tocbdf415CompareMay 27, 2024 14:07
@bradzacherbradzacher added the enhancementNew feature or request labelMay 28, 2024
Copy link
Member

@auvredauvred left a comment

Choose a reason for hiding this comment

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

Left a few minor requests but overall looks great!

Btw, there is no need to ping individuals for review. We have a queue and we get to things when we can.https://typescript-eslint.io/contributing/pull-requests

@auvredauvred added the awaiting responseIssues waiting for a reply from the OP or another party labelMay 30, 2024
@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch 3 times, most recently from7ff0f76 toc13178aCompareJune 3, 2024 13:39
@Vinccool96Vinccool96 requested a review fromauvredJune 3, 2024 16:23
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelJun 3, 2024
@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch froma2715a0 tob121617CompareJune 4, 2024 18:02
@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch 3 times, most recently from40ec555 toe761ee1CompareJune 6, 2024 14:45
@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch frome761ee1 to271d694CompareJune 17, 2024 13:46
@Vinccool96Vinccool96force-pushed thefeat/stricter-rule-test-validations branch from271d694 toc94ed24CompareJune 25, 2024 14:30
@Vinccool96
Copy link
ContributorAuthor

Sorry for the force pushes, I'm used to do rebase updates

@Vinccool96Vinccool96 requested a review fromauvredJune 27, 2024 13:40
@github-actionsgithub-actionsbot removed the awaiting responseIssues waiting for a reply from the OP or another party labelJun 27, 2024
Copy link
Member

@auvredauvred left a comment

Choose a reason for hiding this comment

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

Did you check this comment?#9086 (comment)

We still miss this assertion

@Vinccool96
Copy link
ContributorAuthor

Vinccool96 commentedJun 27, 2024•
edited
Loading

@auvred
Copy link
Member

Onmain there are two different assertions:

constunsubstitutedPlaceholders=
getUnsubstitutedMessagePlaceholders(
message.message,
rule.meta.messages[message.messageId],
error.data,
);
assert.ok(
unsubstitutedPlaceholders.length===0,
`The reported message has${unsubstitutedPlaceholders.length>1 ?`unsubstituted placeholders:${unsubstitutedPlaceholders.map(name=>`'${name}'`).join(', ')}` :`an unsubstituted placeholder '${unsubstitutedPlaceholders[0]}'`}. Please provide the missing${unsubstitutedPlaceholders.length>1 ?'values' :'value'} via the 'data' property in the context.report() call.`,
);

constunsubstitutedPlaceholders=
getUnsubstitutedMessagePlaceholders(
actualSuggestion.desc,
rule.meta.messages[expectedSuggestion.messageId],
expectedSuggestion.data,
);
assert.ok(
unsubstitutedPlaceholders.length===0,
`The message of the suggestion has${unsubstitutedPlaceholders.length>1 ?`unsubstituted placeholders:${unsubstitutedPlaceholders.map(name=>`'${name}'`).join(', ')}` :`an unsubstituted placeholder '${unsubstitutedPlaceholders[0]}'`}. Please provide the missing${unsubstitutedPlaceholders.length>1 ?'values' :'value'} via the 'data' property for the suggestion in the context.report() call.`,
);

@Vinccool96
Copy link
ContributorAuthor

Alright, let me plop it back in

@Vinccool96
Copy link
ContributorAuthor

The prodigal son has returned

@Vinccool96Vinccool96 requested a review fromauvredJune 27, 2024 15:07
auvred
auvred previously approved these changesJun 27, 2024
Copy link
Member

@auvredauvred left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

@auvredauvred added the 1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we merge labelJun 27, 2024
JoshuaKGoldberg
JoshuaKGoldberg previously approved these changesJun 29, 2024
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

The tests for this looked like a real slog, thanks so much for doing this! 🙌

Just a couple of teeny suggestions. Nothing major from me. We can apply them before merging if you don't get a chance before our Monday release.

JoshuaKGoldberg
JoshuaKGoldberg previously approved these changesJun 29, 2024
Copy link
Member

@JoshuaKGoldbergJoshuaKGoldberg left a comment

Choose a reason for hiding this comment

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

Thanks! 🙌

@Vinccool96
Copy link
ContributorAuthor

Had to merge since the main branch had tests that would've failed with the stricter rule tester (tests added by me, what was I thinking!)

@auvredauvred merged commit7e2b77d intotypescript-eslint:mainJul 4, 2024
63 checks passed
@Vinccool96Vinccool96 deleted the feat/stricter-rule-test-validations branchJuly 4, 2024 12:35
@github-actionsgithub-actionsbot locked asresolvedand limited conversation to collaboratorsJul 14, 2024
Sign up for freeto subscribe to this conversation on GitHub. Already have an account?Sign in.
Reviewers

@auvredauvredauvred approved these changes

@JoshuaKGoldbergJoshuaKGoldbergJoshuaKGoldberg left review comments

Assignees
No one assigned
Labels
1 approval>=1 team member has approved this PR; we're now leaving it open for more reviews before we mergeenhancementNew feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Enhancement(rule-tester): Stricter rule test validations
4 participants
@Vinccool96@auvred@JoshuaKGoldberg@bradzacher
[8]ページ先頭
©2009-2025 Movatter.jp