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: dont pin eslint-parser, allow eslint 9 #95

Merged
merged 6 commits into from
Sep 16, 2024

Conversation

Techn1x
Copy link
Contributor

@Techn1x Techn1x commented Sep 11, 2024

I think @babel/eslint-parser might have been mistakenly pinned in this PR #41 to an explicit version instead of using ^

This older version 7.23.10 doesn't allow eslint v9, whereas later minor versions like 7.25.1 do

Screenshot 2024-09-11 at 6 41 10 pm

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 11, 2024

No idea why that no-get test is failing 😢

NullVoxPopuli
NullVoxPopuli previously approved these changes Sep 11, 2024
@NullVoxPopuli NullVoxPopuli added the bug Something isn't working label Sep 11, 2024
@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 13, 2024

I tried a few things to get that no-get test to pass, hoping a dependency had some fixes. No luck there

Decided to make sure the no-get test was passing before my code (even though last main commit was green) - so opened a branch with no diff - turns out that is failing.
#96

So I don't think the updates in this PR are causing the rule failure - something somewhere else has changed.

@NullVoxPopuli
Copy link
Member

Thanks for the analysis!

Any chance you'd be able to fix whatever is going on there?

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 15, 2024

Gotta pin down the root cause & try and get that empty PR passing. Given that the failing step clones eslint-plugin-ember project, which passed a month ago (per your last commit on main branch here), I am guessing that eslint-plugin-ember has changed in some way in that month. After cloning eslint-plugin-ember I'll get the step to checkout older commits of that project and see if it works.

Cloning a project for tests isn't very deterministic, wondering if we should pin it to a commit or version anyway. Something to think about after figuring out the issue

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 16, 2024

Some progress over in #97 - I've left some comments there.

@Techn1x
Copy link
Contributor Author

Techn1x commented Sep 16, 2024

This PR has the fix to get those tests passing now, needed to import the lockfile. Should be good to go.

@NullVoxPopuli NullVoxPopuli merged commit 58cb2d7 into ember-tooling:main Sep 16, 2024
10 checks passed
@NullVoxPopuli
Copy link
Member

Thank you!!

@github-actions github-actions bot mentioned this pull request Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants