search

PhilanthropyDataCommons / service

A project for collecting and serving public information associated with grant applications
GNU Affero General Public License v3.0
8 stars 2 forks source link

Adjust dependabot settings for ESLint 9 #978

Open slifty opened 4 months ago

slifty commented 4 months ago

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.

bickelj commented 4 months ago

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

slifty commented 4 months ago

@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 commented 4 months ago

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 commented 4 months ago

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

slifty commented 4 months ago

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 commented 4 months ago

@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 commented 4 months ago

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 commented 3 months ago

Following up here -- as of May 12th typescript-eslint has (alpha) support for eslint 9: https://github.com/typescript-eslint/typescript-eslint/issues/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 commented 2 months ago

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

slifty commented 2 months ago

@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)!

slifty commented 4 weeks ago

Since opening this issue the typescript-airbnb project has gone into archive mode. I was talking with @jasonaowen about starting a fork of it. The maintainer mentioned someone else might be starting a fork of it as well. Would prefer to contribute to someone else's maintained fork than start one of our own if we can help it!

hminsky2002 commented 4 weeks ago

@slifty any thoughts on using the vercel style guide? https://github.com/vercel/style-guide I came upon this when I was researching style guide projects for mine own projects and it seems fairly regularly maintained. It uses the mozilla public license 2.0 which is weaker than our own, so i'm not sure exactly how integratable with our project it is

slifty commented 4 weeks ago

Since this is just about linting I don't think licensing would have any impact.

That said, I think changing style guides is a Big Deal (it affects what our accepted code style is!) I don't know enough about that style guide and its preferences to know if it'd be a good fit for us, nor if it would have strong opinions on non-vercel concepts.