Skip to content

Conversation

fasttime
Copy link
Member

@fasttime fasttime commented Oct 28, 2024

Prerequisites checklist

What is the purpose of this pull request? (put an "X" next to an item)

[ ] Documentation update
[X] Bug fix (template)
[ ] New rule (template)
[ ] Changes an existing rule (template)
[ ] Add autofix to a rule
[ ] Add a CLI option
[ ] Add something to the core
[ ] Other, please explain:

Update a dependency

fixes #18575

What changes did you make? (Give an overview)

This PR updates @eslint/config-array to version ^0.19.0 and adds unit tests to check that files on different drives are correctly ignored on Windows, which closes #18575.

The changes in lib/eslint/eslint-helpers.js remove unused logic that is probably a leftover of previous refactorings and is not necessary when paths (including glob patterns) are resolved to absolute paths. The changes in RuleTester should not be user-facing.

Is there anything you'd like reviewers to focus on?

Am I missing any unit tests?

@eslint-github-bot eslint-github-bot bot added the build This change relates to ESLint's build process label Oct 28, 2024
@github-actions github-actions bot added the core Relates to ESLint's core APIs and features label Oct 28, 2024
Copy link

netlify bot commented Oct 28, 2024

Deploy Preview for docs-eslint canceled.

Name Link
🔨 Latest commit 31938be
🔍 Latest deploy log https://app.netlify.com/sites/docs-eslint/deploys/672b11a1316d9c0008467177

@github-actions github-actions bot added the cli Relates to ESLint's command-line interface label Nov 3, 2024
@fasttime fasttime changed the title build: update @eslint/config-array to v0.19.x fix: ignore files on a different drive on Windows Nov 3, 2024
@eslint-github-bot eslint-github-bot bot added the bug ESLint is working incorrectly label Nov 3, 2024
@mdjermanovic mdjermanovic added accepted There is consensus among the team that this change meets the criteria for inclusion and removed build This change relates to ESLint's build process labels Nov 3, 2024
Comment on lines -1335 to -1350
it("should allow setting the filename to a file path without extension", () => {
ruleTester.run("", require("../../fixtures/testers/rule-tester/no-test-filename"), {
valid: [
{
code: "var foo = 'bar'",
filename: "somefile"
},
{
code: "var foo = 'bar'",
filename: "path/to/somefile"
}
],
invalid: []
});
});

Copy link
Member Author

Choose a reason for hiding this comment

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

I integrated this test into another one with the same title further below.

@fasttime fasttime marked this pull request as ready for review November 4, 2024 16:18
@fasttime fasttime requested a review from a team as a code owner November 4, 2024 16:18
mdjermanovic
mdjermanovic previously approved these changes Nov 5, 2024
Copy link
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

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

LGTM, thanks! Would like @nzakas to verify before merging.

let otherDriveLetter;
const exec = util.promisify(require("node:child_process").exec);

before(async () => {
Copy link
Member

Choose a reason for hiding this comment

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

Can you add a comment here explaining what's happening? I'm not quite sure I get it now, and I'm sure in another year I'll have no idea. 😄

Copy link
Member Author

Choose a reason for hiding this comment

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

I added comments in 31938be. Hope it's clearer now.

Copy link
Member

@nzakas nzakas left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks.

@nzakas nzakas merged commit 68fa497 into main Nov 6, 2024
26 checks passed
@nzakas nzakas deleted the update-for-rewrite-pr-59 branch November 6, 2024 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

accepted There is consensus among the team that this change meets the criteria for inclusion bug ESLint is working incorrectly cli Relates to ESLint's command-line interface core Relates to ESLint's core APIs and features

Projects

Status: Complete

Development

Successfully merging this pull request may close these issues.

Bug: files on a different drive are not ignored on Windows

3 participants