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

Switching message on success to "Successfully <past>" #798

Closed
wants to merge 2 commits into from

Conversation

ThomasRot
Copy link

Just a minor grammar fix as to my knowledge "succeed to " does not exist..

@github-actions github-actions bot added bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) scripts related to shell scripts (bash and CMD) labels Jun 20, 2022
@hohwille
Copy link
Member

hohwille commented Jun 23, 2022

@ThomasRot thanks for your PR and contributing to devonfw-ide.
Could you explain the rationale for your change?

Just a minor grammar fix as to my knowledge "succeed to " does not exist..

It actually is Succeeded to :

doSuccess "Succeeded to ${message}"

Further there are multiple places for this (you only changed one of them):
doSuccess "Succeeded to ${1}"

I checked the grammar:
https://www.collinsdictionary.com/dictionary/english/succeed

So it seems Succeeded is correct and Succeeded to also exists, but is not the correct form here as in Succeeded to the Succeeded means "inherited" like in

George III succeeded to the throne in 1760.

So actually correct grammar would be Succeeded in + verb in infinitive:

Succeeded in doing something

I was not aware of this detail of English grammar so thanks for your correction.

However, as we also use the message in present form for debug output before executing the command it also does not make sense to change it to the past form everywhere.
Instead I would simply change the message from Succeeded to to something more simple like Success: or Successfully performed .

@hohwille hohwille added the waiting-for-feedback we have asked a question and the issue is pending until an answer is provided label Jun 23, 2022
@hohwille
Copy link
Member

I have created PR #802 as a simple fix and replacement for this PR.
If OK, I would close this PR and merge #802 to resolve the actual issue.

@hohwille
Copy link
Member

@ThomasRot beginning of next week I will close this PR and merge #802 unless you have anything else to report. We try to make continuous progress and releases (again after a time where I was M.I.A.).

@ThomasRot
Copy link
Author

All good points especially as this change did not consider all implications. I like the solution proposed in #802, so I'm closing this one.

@ThomasRot ThomasRot closed this Jun 24, 2022
@hohwille hohwille added this to the rejected milestone Aug 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bash related to bash shell or scripts commandlet related to commandlets (scripts/command/*) scripts related to shell scripts (bash and CMD) waiting-for-feedback we have asked a question and the issue is pending until an answer is provided
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants