Skip to content

Conversation

Copy link
Contributor

Copilot AI commented Dec 30, 2025

Applies review feedback from PR #344 to improve CPU feature detection error handling and efficiency.

Changes:

  • Memoize isPortable() result - Cache the detection result in a module-level variable to prevent redundant require('cpu-features') attempts and duplicate warning logs when called multiple times during binary download/installation

  • Add error object to warning log - Pass the caught error to logger.warn() for better debugging visibility when CPU feature detection fails

// Before
catch (error) {
  logger.warn('Failed to detect CPU features, using portable binary');
  return true;
}

// After  
let cachedIsPortable: boolean | undefined = undefined;

function isPortable(): boolean {
  if (cachedIsPortable !== undefined) {
    return cachedIsPortable;
  }
  
  try {
    // ... detection logic sets cachedIsPortable ...
  } catch (error) {
    logger.warn('Failed to detect CPU features, using portable binary', error);
    cachedIsPortable = true;
  }
  
  return cachedIsPortable;
}

✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: RetricSu <23436060+RetricSu@users.noreply.github.com>
Copilot AI changed the title [WIP] Use portable if cpu-feature build fails Memoize isPortable() and add error logging Dec 30, 2025
Copilot AI requested a review from RetricSu December 30, 2025 02:48
@RetricSu RetricSu marked this pull request as ready for review December 30, 2025 02:49
@RetricSu RetricSu merged commit f091b5f into fix-window-install-failed Dec 30, 2025
1 check failed
@RetricSu RetricSu deleted the copilot/sub-pr-344 branch December 30, 2025 02:49
RetricSu added a commit that referenced this pull request Dec 30, 2025
* use portable if cpu-feature build fails

* fix pnpm-lock

* fix isPortable

* Memoize isPortable() and add error logging (#345)

* Initial plan

* Apply review suggestions: add error logging and memoize isPortable

Co-authored-by: RetricSu <23436060+RetricSu@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: RetricSu <23436060+RetricSu@users.noreply.github.com>

* chore: fmt

---------

Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: RetricSu <23436060+RetricSu@users.noreply.github.com>
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

Successfully merging this pull request may close these issues.

2 participants