Reconfigure/disable renovate about npm-search HOT 8 OPEN

(Oh also, I could disable Dependabot. Maybe it will be automatically re-enabled again, we'll see.)

from npm-search.

I'm not overly eager to update everything here (especially in an automated way) because the test coverage isn't too great, and if something breaks, the likely solution is a full reindex, which is still slow.

Specifically with hosted-git-info, I did try to update it because the current version throws exceptions for some invalid URLs, but then as I ran into more issues, I decided to just add a try/catch instead. Swapping the order actually might work, but I wasn't 100% confident it won't break some other cases that we don't test.

As for ESM, it's theoretically just an option in the TypeScript config, but realistically, it also impacts production monitoring (APM instrumentation support), running tests/coverage, and some more advanced mocking scenarios. The ecosystem support is fairly good at this point, and I think on this project, the impact should be low, but so is the motivation to do it - all of the Sindre's projects work fine in their older CJS versions, and there is close to zero advantage in running this as ESM at this point.

got could be replaced by the native fetch imo

While I'm in general a fan of using built-in staff, the fetch is very limited compared to got, and its API is a little awkward. For purely backend projects, I see no good reason to use it.

from npm-search.

@Haroenv The hosted-git-info is hosted under npm/hosted-git-info, and last commit is September 18th, 2023, so it seems to be active and what npm is using. (https://github.com/npm/hosted-git-info)

Would you had another in mind they could be actually using?

from npm-search.

Yeah, I'm sorry about that, it shouldn't be that way. It was correctly updating dependencies in the background and grouping them into one PR. Then I disabled CircleCI, and it opened the floodgates.

The Renovate setup wasn't the one I anticipated so I didn't see that coming. I assumed it was doing silent updates on branches based on master, while it is actually doing PR updates on chore/renovateBaseBranch.

I can't really disable Renovate completely as I kinda need it running to configure/debug it. All I can suggest is muting the notifications for this repo while I work on the fix 🙏

from npm-search.

Renovate should be going more smoothly now. It should be back on its initial configuration of creating PRs to merge on chore/renovateBaseBranch, and we decide to merge this branch on master when we need it (I just did it).

But I'd like to discuss more broadly about how you (@Haroenv and @MartinKolarik, and even @bodinsamuel if you'd like to chime in) think we should handle dependency updates.

I'm coming from a background where I try to automate dependency updates as much as I can. If it does not break the lint/test/build process, then I usually let Renovate auto-merge any minor or patch upgrade (I keep any major update under manual review). The rationale is that when I need to upgrade a package (and it's a when, not an if IMO), I don't have a very hard migration from 5 major versions ago where it's hard to pinpoint exactly what fails (because everything fails at the same time).

On this project though, I've come to realize we have taken a more "if it ain't broken, don't fix it" approach with some packages (as we've seen with the git-hosted-info or got packages that work fine and whose upgrade require some work that might not be worth it), and I totally understand the rationale behind it. A regression not caught by our tests might impact thousands of packages in the index.

Whatever we decide to do, I think we should be a bit more explicit about our thought process, and write it down. At least discuss it here between us, maybe write some guidelines in the documentation and configure renovate accordingly.

To ground the discussion a little bit more, what are your thoughts on the two following packages:

• got. Upgrading means apparently moving the whole project to ESM. I've never done that before, and it seems like a lot of work, but also something we'll eventually have to do to keep up with the JS ecosystem. Is today too soon? When will it be the right time?

• hosted-git-info. We're on v2.7. v2.8 broke our script, and now v7 is out. We currently use it to extract repository metadata from a URL, and if it fails (because it does not handle tree URLs, only root URLs) we fallback to our own implementation. I did a quick POC and swapping the order (first testing our own implementation, and if it fails use hosted-git-info) seem to work (all tests are green). Should we upgrade with this change? Are there unforeseen consequences? Should we pin this dep to 2.7 forever instead?

You have much more experience with the code here than I have, so I highly respect your opinions on the matter and am looking forward to them

Edit: Also, do we really need Dependabot if we have Renovate?

from npm-search.

I will summarise our approach regarding renovate strategy since it might not be clear from the outside world.
Upgrading dependencies is slow and painful, especially when you are maintaining hundreds of repo. In my previous team we were managing dozens and it was really impossible to test everything.
To "help" we decided to introduce the system of weekly branch merge:

• Every friday a branch and a PR is created
• Renovate runs on the weekend to avoid slowing down business critical CI (shared runners)
• On monday you review and merge

Why not merging directly?

Because, and especially in this repo, some parts are not tested or hardly testable. So a broken update can enter the main and it's even more painful to test, to make a PR to revert, etc. For something this is not always urgent.
And it creates only one commit in main which limits the noise, is easy to revert, test and point in the history.

The downsides

It requires at least one human intervention (can be weekly or less often though)
If nobody merge the branch, it will be updated indefinitely and main will still be outdated.

Keeping this strategy is up to you, I don't have strong opinion it just served us very well during the years.

Regarding the broken updates, I think it's fair to say we (you) need to do something:

• I had left hosted-git-info on the side for 2 years but it's problematic because everything runs in a live production. If somebody find a security hole and publish a package that exploits it, it could be a real issue (could already be the case). Most people disregard upgrading because they are only managing libraries / local env, but it's a different topic with live environment.

• got could be replaced by the native fetch imo

• ESM migration, while I don't enjoy the way it was pushed onto the Node env and really struggled to use it for years, it has become easier with Typescript. It would still needs a few hours to migrate I think. And could help for other packages upgrade.

I think Dependabot is forced at the Org level, you can't disable it.

from npm-search.

Summarizing the points above, what I think we can do is:

• Document the current semi-manual update strategy in CONTRIBUTING.md.
  We seem to all agree that full automated update is too dangerous here, and this strategy has worked well for years, so we can probably keep it. Documenting it so we explicit the rationale.

• Carefully update hosted-git-info to the latest version.
  As mentioned, there is a security risk in staying on the old version forever. But also a production risk upgrading because of the low coverage. I have a branch where I updated the version, adapted the code, and have all tests green. As an added precaution, I will also compare this rewrite against what is in the current index and see if it would have diverged. I will add any difference as new test cases, so we should be covered.

• Keep got and pin it to the pre-ESM version in renovate config
  We'll eventually have to move to ESM, but let's wait until we have the whole run stack back in good shape (pending GCP / Datadog integrations). In the meantime we can tell renovate to ignore those packages, to limit the noise, and we'll upgrade them all once the time is right.

Did I miss anything?

from npm-search.

As mentioned, there is a security risk in staying on the old version forever

The security risk is just a ReDos, not applicable in our case. If we'd do anything I'd migrate to what npm uses (a different package if I remember it correctly) or vendor the code

from npm-search.