Skip to content
New issue

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

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

Already on GitHub? # to your account

Add descriptions for some freerdp exit status code #1101

Merged
merged 2 commits into from
Jan 30, 2017
Merged

Conversation

giox069
Copy link
Contributor

@giox069 giox069 commented Jan 30, 2017

@antenore: I added 4 new text strings for these errors. Can you have them reviewed by a English mother tongue technician ?

@antenore
Copy link
Member

Sure.

@antenore
Copy link
Member

antenore commented Jan 30, 2017

They are perfect. Do I merge? Do you have anything to add?

EDIT: Another opinion. Being scrupulous "Account is expired" and "Account is disabled" would be better, but "Account expired" and "Account disabled" it's not wrong at all.

@giox069
Copy link
Contributor Author

giox069 commented Jan 30, 2017

I will add "is", but check also for "has expired" vs "is expired". Which one ?

@antenore
Copy link
Member

antenore commented Jan 30, 2017

In general, you should use "to be" or "to have" in accordance of the first verb in a sentence, for example:

I have checked and the account has already expired (have -> has).

Another thing is that "is expired" implies a long-standing condition (The account is expired sine 2 years), "has expired" a recent change in the state (the account has just expired).

So, "is expired", as we don't know when, should be the most appropriate. Than I don't think we have several Oxford users to care so much in this case ;-)

Go for "is expired", "is locked out" etc.

Thank you Sir. Beckett!!

@giox069
Copy link
Contributor Author

giox069 commented Jan 30, 2017

@antenore: it should be fine to merge now.
This patch adds 4 possible error return code from libfreerdp. But not all four seems to be currently implemented inside libfreerdp. I was only able to test and have good results only with STATUS_ACCOUNT_DISABLED and STATUS_ACCOUNT_LOCKED_OUT.

@antenore
Copy link
Member

Perfect, really useful. I'm merging it.

@antenore antenore merged commit 02787c5 into next Jan 30, 2017
@giox069 giox069 deleted the more_rdp_errors branch February 12, 2017 12:01
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants