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

maint(pam/nativemodel): Improve wording and consistency of strings #599

Merged
merged 6 commits into from
Nov 6, 2024

Conversation

3v1n0
Copy link
Collaborator

@3v1n0 3v1n0 commented Oct 18, 2024

Use the same terminology and similar behavior of the CLI mode

UDENG-5012

@3v1n0 3v1n0 requested a review from a team as a code owner October 18, 2024 21:53
@3v1n0 3v1n0 force-pushed the native-wording-improvements branch from 4c3c4f7 to 9087e8d Compare October 18, 2024 21:59
@codecov-commenter
Copy link

codecov-commenter commented Oct 18, 2024

Codecov Report

Attention: Patch coverage is 82.05128% with 14 lines in your changes missing coverage. Please review.

Project coverage is 82.93%. Comparing base (ab5a98a) to head (2de0fe2).

Files with missing lines Patch % Lines
pam/internal/adapter/nativemodel.go 82.05% 11 Missing and 3 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #599      +/-   ##
==========================================
+ Coverage   82.86%   82.93%   +0.07%     
==========================================
  Files          80       80              
  Lines        8514     8534      +20     
  Branches       75       75              
==========================================
+ Hits         7055     7078      +23     
+ Misses       1129     1127       -2     
+ Partials      330      329       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

pam/internal/adapter/nativemodel.go Outdated Show resolved Hide resolved
pam/internal/adapter/nativemodel.go Outdated Show resolved Hide resolved
pam/internal/adapter/nativemodel.go Outdated Show resolved Hide resolved
pam/internal/adapter/nativemodel.go Outdated Show resolved Hide resolved
@3v1n0 3v1n0 force-pushed the native-wording-improvements branch 2 times, most recently from 07cc197 to 6a23cee Compare October 22, 2024 00:36
@3v1n0 3v1n0 requested a review from adombeck October 22, 2024 00:36
@3v1n0 3v1n0 force-pushed the native-wording-improvements branch from 6a23cee to 651c697 Compare October 22, 2024 10:24
@3v1n0 3v1n0 requested a review from adombeck October 22, 2024 10:24
@3v1n0 3v1n0 force-pushed the native-wording-improvements branch from 651c697 to ca979f1 Compare October 22, 2024 11:32
@3v1n0 3v1n0 requested a review from adombeck October 22, 2024 11:32
@3v1n0 3v1n0 force-pushed the native-wording-improvements branch from ca979f1 to 5de61a1 Compare October 22, 2024 11:39
Copy link
Member

@denisonbarbosa denisonbarbosa left a comment

Choose a reason for hiding this comment

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

Some things you just don't know you need it until you get it 😂

Thanks for this. Nice work!

@3v1n0 3v1n0 requested a review from adombeck November 4, 2024 21:15
3v1n0 added 4 commits November 5, 2024 21:30
Use the same terminology and similar behavior of the CLI mode
We were trying to fix the broker behavior here, but it's something that
was done only by the native UI and it's not really something used
anywhere.

So instead just error in case this happens, since we expect to have
defined labels here.

In case we will ever require to have a fallback string, this should be
done at general level and not just at PAM level.
Make it clearer where a "go back" action will lead the user to.
Use a behavior that is more similar to what we've in the CLI interface
so that when an input is requested the `>` char is used.
@3v1n0 3v1n0 force-pushed the native-wording-improvements branch from 5de61a1 to f55d343 Compare November 5, 2024 23:02
In native modules the prompt should be only the text shown on the
field, while the rest of information should be exposed as PAM info
messages.

Do this to ensure that both all PAM implementations we care about (ssh
and polkit) keep a reasonable UI.
@3v1n0 3v1n0 force-pushed the native-wording-improvements branch from f55d343 to 79f21be Compare November 5, 2024 23:17
@3v1n0 3v1n0 merged commit c91dd5b into ubuntu:main Nov 6, 2024
8 checks passed
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