Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cli: don't error if the user folder doesn't exist #4508

Merged
merged 1 commit into from
Apr 12, 2022

Conversation

brunnre8
Copy link
Member

@brunnre8 brunnre8 commented Mar 8, 2022

The user folder gets created on demand, thelounge list should not
fail if the folder doesn't exist.
This just means that no users are present, so report that instead.

The user folder gets created on demand, thelounge list should not
fail if the folder doesn't exist.
This just means that no users are present, so report that instead.
log.info(
`There are currently no users. Create one with ${colors.bold(
"thelounge add <name>"
)}.`
);
return;
Copy link
Member Author

Choose a reason for hiding this comment

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

I'm a fan of return early, keep the happy path to the left.
If you disagree heavily please tell me and I'll adapt my style.

Copy link
Member

Choose a reason for hiding this comment

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

I think either way is fine. Code style seems like a fairly low stakes thing in an OSS code case with an auto formatter too.

Copy link
Member

@itsjohncs itsjohncs left a comment

Choose a reason for hiding this comment

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

LGTM. Didn't test this myself but the code paths are straightforward so I think the testing you've done in development should be sufficient.

@itsjohncs itsjohncs self-assigned this Mar 9, 2022
@itsjohncs itsjohncs added the Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors. label Mar 9, 2022
@brunnre8 brunnre8 added the Status: release-after-next reviewed and ready to merge, postponed to release after next (feature freeze of current release) label Mar 9, 2022
@itsjohncs itsjohncs added this to the 4.3.2 milestone Mar 10, 2022
@MaxLeiter MaxLeiter removed the Status: release-after-next reviewed and ready to merge, postponed to release after next (feature freeze of current release) label Apr 12, 2022
@MaxLeiter MaxLeiter merged commit 8153198 into master Apr 12, 2022
@MaxLeiter MaxLeiter deleted the bookworm/userList branch April 12, 2022 00:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug Issues that report and PRs that solve any defects that cause unexpected behaviors.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants