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

Adjust dependabot settings for ESLint 9 #978

Open
1 task done
slifty opened this issue May 1, 2024 · 10 comments
Open
1 task done

Adjust dependabot settings for ESLint 9 #978

slifty opened this issue May 1, 2024 · 10 comments
Assignees

Comments

@slifty
Copy link
Member

slifty commented May 1, 2024

Right now ESlint 9 isn't supported by the typescript-eslint package

The first step is to tell dependabot to ignore major eslint versions, and then we wait on the following to get closed:

Once that's done, we should remove the dependabot ignore rule.

slifty added a commit that referenced this issue May 1, 2024
Right now our typescript-eslint dependency does not support eslint v9.
They're working on it [1] but until that's done we want dependabot to
ignore major eslint updates.

Issue #978 Adjust dependabot settings for ESLint 9

[1] typescript-eslint/typescript-eslint#8211
@bickelj
Copy link
Contributor

bickelj commented May 1, 2024

Can Dependabot or GitHub notify us once that issue is closed?

@slifty
Copy link
Member Author

slifty commented May 1, 2024

@bickelj I don't know! I've never used a tracking issue before, but otherwise I think we'll just need to check back every once in a while (I expect to be on top of it since most of the projects I work on are in the same boat)

@slifty
Copy link
Member Author

slifty commented May 1, 2024

Oh, another thought: when typescript-eslint update to support eslint 9 I have a very strong feeling it will require eslint 9, and so we'll naturally be informed by dependabot (in that the PR will break, and the logs will show we have to update eslint to v9) at which point we'll know what to do.

@bickelj
Copy link
Contributor

bickelj commented May 2, 2024

@slifty I'm still not convinced it's better, but it's also not a big deal, therefore I yield.

@slifty
Copy link
Member Author

slifty commented May 2, 2024

In case it helps: I think this is what the dependabot ignore feature was made for (disabling dependabot for things we are not interested in updating based on current state). We'll end up being alerted the moment the dependency is ready for dependabot again thanks to the very peer dependencies that are breaking things now.

Given all that, I don't quite grok the downside (whereas I see a very blatant downside in getting multiple alerts a week about un-mergable dependabot PRs for the next few months).

@bickelj
Copy link
Contributor

bickelj commented May 2, 2024

@slifty My understanding of "ignore this major version of X dependency" was that dependabot wouldn't attempt to upgrade X to any of those major versions. I didn't know that with that ignore rule being committed that somehow dependabot could know that we actually want that major version or have a way to detect when we're ready for it. If it can do that, great. I don't think you're suggesting that it can, though. I think your feeling above may be right, namely that other ESLint-related dependencies will alert us because they do not have backward compatibility with their new versions. I suppose we can lean on that and/or manual checking of the respective issues. The downside is that we don't get alerted to a compatible ESLint 9, which I assume we want.

Contrast the case of another dependency we told dependabot to ignore for a major version, mockjwks. We decided we don't like ESM but that major version uses ESM. So we do not want that major version now nor in the near future. We could revisit if we decide we like ESM. In your phrasing, yes, it falls under the same umbrella of "temporarily we don't want this thing" but I would categorize it a bit differently. The only reason we don't want ESLint 9 now is because other dependencies related to it break with it.

Anyway, there is more than one way to do it. I'm OK with this other way but in general I would rather mash "close" a few times on failed upgrade attempts so that I can get the alert when the upgrade attempt works. If we get it anyway due to some implementation- or library-specific knowledge, great, this is a special case.

@slifty
Copy link
Member Author

slifty commented May 2, 2024

Ah got ya! Appreciate the further explanation, and I think I now understand the shape of the general concern and that this is banking on something we can't truly rely on in terms of reminding us to reverse course down the line.

@slifty slifty self-assigned this May 16, 2024
@slifty
Copy link
Member Author

slifty commented Jun 18, 2024

Following up here -- as of May 12th typescript-eslint has (alpha) support for eslint 9: typescript-eslint/typescript-eslint#8211

It's still in alpha, but once ts-eslintv8 is officially released we'll get an update request and at that point we'll know we have to update it's peer dependencies (aka eslint 9)

@bickelj
Copy link
Contributor

bickelj commented Jun 24, 2024

Checking again, as of today June 24, 2024, typescript-eslint 8 is still alpha.

@slifty
Copy link
Member Author

slifty commented Jun 24, 2024

@bickelj we don't need to bump this thread -- we'll get a dependabot notification the day it goes out of alpha (we aren't ignoring typescript-eslint major updates, just the baseline eslint)!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants