Skip to content

Conversation

@otaviojacobi
Copy link
Contributor

Change-type: patch

Resolves: #
Change-type: major|minor|patch
Depends-on:
See:


Please check the CONTRIBUTING.md file for relevant information and some
guidance. Keep in mind that the CLI is a cross-platform application that runs
on Windows, macOS and Linux. Tests will be automatically run by balena CI on
all three operating systems, but this will only help if you have added test
code that exercises the modified or added feature code.

Note that each commit message (currently only the first line) will be
automatically copied to the CHANGELOG.md file, so try writing it in a way
that describes the feature or fix for CLI users.

If there isn't a linked issue or if the linked issue doesn't quite match the
PR, please add a PR description to explain its purpose or the features that it
implements. Adding PR comments to blocks of code that aren't self explanatory
usually helps with the review process.

If the PR introduces security considerations or affects the development, build
or release process, please be sure to highlight this in the PR description.

Thank you very much for your contribution!

Change-type: patch
@otaviojacobi otaviojacobi requested a review from a team October 17, 2025 17:16
@flowzone-app flowzone-app bot enabled auto-merge October 17, 2025 17:58
Comment on lines +154 to +161
await Promise.race([
Promise.all([trackPromise, deprecationPromise, runPromise]),
runPromise.then(async () => {
if (deprecationPromise) {
await deprecationPromise;
}
}),
]);
Copy link
Member

@thgreasi thgreasi Oct 20, 2025

Choose a reason for hiding this comment

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

It looks equivalent to this v

Suggested change
await Promise.race([
Promise.all([trackPromise, deprecationPromise, runPromise]),
runPromise.then(async () => {
if (deprecationPromise) {
await deprecationPromise;
}
}),
]);
void trackPromise;
await runPromise;
if (deprecationPromise) {
await deprecationPromise;
}

and the void trackPromise; shouldn't even be necessary, since should be a side-effect of await import('./hooks/prerun'); anyway. ie we probably don't need to import any specific export of ./hooks/prerun.


await Promise.all([trackPromise, deprecationPromise, runPromise]);
// Help command and others may not trigger the prerun hook, so we need
// to handle the case where trackPromise never resolves. We use Promise.race
Copy link
Member

Choose a reason for hiding this comment

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

I would argue that trackResolve should always settle to something, and how we have it atm isn't really great. It seems that we really care about is whether we need to still wait for tracking events to be sent, and not whether an event was really sent or not w/ the current command. This way it seems that trackResolve should be in a finally block in the hooks, so that it's always called even if the currently issued command didn't call the tracking code at all.

archive)
build build a project locally
deploy deploy a single image or a multicontainer project to a
balena fleet
Copy link
Member

Choose a reason for hiding this comment

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

This doesn't sound right

util available-drives list available drives
version display version information for the balena CLI and/or Node.js
whoami display account information for current user
api-key generate generate a new balenaCloud API key
Copy link
Member

Choose a reason for hiding this comment

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

I guess you had to manually copy paste this all here? That does not sound very maintainable, and would require a change to this file following many PRs. It may be nice if we can just have npm run build send help output to a txt file in the tests folder and we use that for the tests

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.

4 participants