Skip to content

Conversation

@DonJayamanne
Copy link
Contributor

cp.exec(command, (err, stdout) => {
if (err) {
traceVerbose(`Error running command: ${command}`, err);
traceError(`Error running command: ${command} (${timer.elapsedTime})`, err);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Surely if success is info, then failure must be warning/error or info, but not something less

return available.promise;
}

log?.info(`Running: uv --version`);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think the code is right here, as log is optional
And looking at the calling code, it isn't passed

Not sure why, I'd try to resolve this in debt, as logger shouldn't be optional. we either turn off logging or the like, but wouldn't make the logger component optional.

const deferred = createDeferred<string>();
args = quoteArgs(args);
const timer = new StopWatch();
deferred.promise.finally(() => traceInfo(`Ran conda in ${timer.elapsedTime}: ${conda} ${args.join(' ')}`));
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't logged this at all, now we will.
We know conda is slow

const exec = promisify(cp.exec);
function execCommand(command: string) {
const timer = new StopWatch();
return promisify(cp.exec)(command, { windowsHide: true }).finally(() =>
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We haven't logged this at all, now we will.
I believe we've seen delays in reading registry in the past as well.

@DonJayamanne DonJayamanne requested review from eleanorjboyd and karthiknadig and removed request for karthiknadig August 23, 2025 01:02
@DonJayamanne DonJayamanne merged commit ca8d03a into main Aug 25, 2025
10 checks passed
@DonJayamanne DonJayamanne deleted the don/furious-mandrill branch August 25, 2025 20:13
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.

3 participants