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

add username prompt, reservedUsernames config, minor cleanup #22

Merged

Conversation

nmschulte
Copy link
Contributor

I created this work for Mobian, and as suggested am sharing here for review and inclusion. A question is open about where the reserved usernames list comes from; see https://salsa.debian.org/Mobian-team/packages/calamares-extensions/-/merge_requests/4.

... Sharing my WIP for early review; should the username prompt be a separate screen, or is it acceptable to add it to the password prompt screen?
I also wonder how to properly attribute my work ...

@nmschulte nmschulte changed the title Nms/username prompt origin Draft: add username prompt, reservedUsernames config, minor cleanup Sep 22, 2022
@adriaandegroot
Copy link
Contributor

I don't have a particular comment to make here, since the module is mobile-(mobian)specific and I don't feel qualified to comment. Maybe @ollieparanoid or @PureTryOut ? Those are Postmarket folks, I think.

I don't know what the source is of the reserved names. It seems to contain a bunch that I wouldn't call reserved in the context of a general OS (cyrus, for instance, unless cyrus imap is always on the system). Moving the reserved list to C++ makes it much harder to adapt to whatever mobile OS is using this module, though.

@PureTryOut
Copy link

It seems fine, although I agree on the reserved usernames comment.
The module is mobile-specific, but not Mobian-specific 😉

@nmschulte
Copy link
Contributor Author

I'll fix the TODO about the validation. Any thoughts about the placeholder TODO?
The reserved usernames change allows them to be configurable -- I can neuter the default list in the C++ code which IIUC is required though I'm not familiar with the system but came up with this from the other patterns; recommendations?

Copy link
Contributor

@ollieparanoid ollieparanoid left a comment

Choose a reason for hiding this comment

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

LGTM

@nmschulte nmschulte changed the title Draft: add username prompt, reservedUsernames config, minor cleanup add username prompt, reservedUsernames config, minor cleanup Feb 20, 2023
@adriaandegroot adriaandegroot merged commit 2dd9a7b into calamares:calamares Feb 24, 2023
@nmschulte nmschulte deleted the nms/username-prompt-origin branch February 24, 2023 21:36
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