-
Notifications
You must be signed in to change notification settings - Fork 28
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
Replace "transfer" notion with "convert" notion #1228
Conversation
34b26cb
to
d2954e2
Compare
@Pytal (you or another possible reviewer) I need you to test ; thanks by advance. |
4364809
to
7c8c65d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems good to me – I only don’t know how the more complex constructions like this one would come out:
$l->t('Conversion of guest {guest} account to {user} account completed'),
The previous simpler way without the 2 "account" wordings seems nicer? But @Jerome-Herbinet if you have examples of before/after how it will actually come out with example wording that would be nice.
@jancborchardt you are right ; the sentence is a bit complex, but I think that giving enough details is necessary in this process. I would even prefer, even it's longer (guest vs. regular) :
... and to apply to the other wordings. |
@Pytal since you are more pro at English, what do you suggest? :D I would prefer to go with a simple and concise sentence as that often is more understandable. |
Both ways have valid points I think — considering the messages in isolation and without assuming contextual knowledge I would say the more explicit messaging makes sense |
7c8c65d
to
d6334bd
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Jerome-Herbinet could you still show how these would come out in practice?
$l->t('Failed to convert guest {guest} account to {user} account')
$l->t('Conversion of guest {guest} account to {user} account completed')
d6334bd
to
c1c3176
Compare
I won't be able to test it @jancborchardt |
@Pytal do you know what will come out of those sentences? :D It would be important to know so we can decide on the proper wording. |
@jancborchardt looks like this |
With the longer version |
Signed-off-by: Jérôme Herbinet <[email protected]> Replace "transfer" notion with "convert" notion Signed-off-by: Jérôme Herbinet <[email protected]> Replace "transfer" notion with "convert" notion Signed-off-by: Jérôme Herbinet <[email protected]> Update TransferGuestDialog.vue Signed-off-by: Jérôme Herbinet <[email protected]> Update lib/Notifications/Notifier.php Signed-off-by: Jérôme Herbinet <[email protected]> Update lib/Notifications/Notifier.php Signed-off-by: Jérôme Herbinet <[email protected]> Update src/components/TransferGuestDialog.vue Signed-off-by: Jérôme Herbinet <[email protected]> Update src/components/TransferGuestDialog.vue Signed-off-by: Jérôme Herbinet <[email protected]> Co-Authored-By: Pytal <[email protected]>
c1c3176
to
63f5900
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @Pytal! :) I think it is a bit cumbersome and would be way better with both occurences of "account" deleted, so it is like this:
Conversion of guest [email protected] to (test1) completed
Especially as the proper account has the userbubble then, it’s way easier to read.
Signed-off-by: Jérôme Herbinet <[email protected]>
Signed-off-by: Jérôme Herbinet <[email protected]>
@jancborchardt check my 2 last commits. |
Good stuff, thanks @Jerome-Herbinet and sorry for the back and forth :) |
/backport to stable30 |
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Replace "transfer" notion with "convert" notion
Test from reviewers needed.
Fixes: #1221 (except menu entries order which is IMO another development … that I can't do myself)