Skip to content

Conversation

rgrunber
Copy link
Member

@rgrunber rgrunber commented Sep 18, 2025

  • When the Devfile Registries list contains other registries, it should be permitted to remove the Default Devfile Registry

  • Replace usage of https.get(..) with fetch

  • Need to test this more thoroughly

  • I can probably adjust

    https.get(`${url}/api`, options, (response) => {
    as well

@vrubezhny
Copy link
Contributor

  • Replace usage of https.get(..) with fetch

Some time ago we'd refused using fetch because it was bringing incompatibilities between make-fetch-happen versions brought by vscode itself and the extensions and their dependencies that bring their own fetch version.

See: #4233

Also, AFAIR, the fetch version used by vscode wasn't providing AbortController support, and this was yet another reason we'd got rid of using it.

@rgrunber
Copy link
Member Author

make-fetch-happen was removed right after that, at #4247 . Is this still an issue ?

@vrubezhny
Copy link
Contributor

make-fetch-happen was removed right after that, at #4247 . Is this still an issue ?

Nope, but you're trying to bring it back with your PR

@rgrunber rgrunber force-pushed the devfile-registry-fixes branch from 2fc404d to 6ed0df4 Compare September 18, 2025 15:31
@rgrunber
Copy link
Member Author

Ok, I've gone with just switching between http/https.

- When the Devfile Registries list contains other registries, it should
  be permitted to remove the Default Devfile Registry
- Switch between http/https.get(..) depending on URL protocol

Signed-off-by: Roland Grunberg <[email protected]>
@rgrunber rgrunber force-pushed the devfile-registry-fixes branch from 6ed0df4 to b14f74f Compare September 18, 2025 15:35
Copy link

codecov bot commented Sep 18, 2025

Codecov Report

❌ Patch coverage is 76.92308% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 44.62%. Comparing base (da60441) to head (b14f74f).
⚠️ Report is 1324 commits behind head on main.

Files with missing lines Patch % Lines
src/openshift/cluster.ts 50.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #5361       +/-   ##
===========================================
+ Coverage   32.37%   44.62%   +12.25%     
===========================================
  Files          85      100       +15     
  Lines        6505     8831     +2326     
  Branches     1349     1949      +600     
===========================================
+ Hits         2106     3941     +1835     
- Misses       4399     4890      +491     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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

Successfully merging this pull request may close these issues.

2 participants