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

Changed should to must in exception messages #17710

Merged
merged 1 commit into from
Oct 19, 2020

Conversation

ndeevy
Copy link

@ndeevy ndeevy commented Oct 10, 2020

Resolves #11723

Replaced the word 'should' with (in most cases) 'must' in exception messages. This is to improve user focus and to indicate obligation rather than advice.

@ndeevy ndeevy requested a review from a team as a code owner October 10, 2020 13:29
@ghost ghost added the Source: Community PR or issue was opened by a community user label Oct 10, 2020
@devOpsHazelcast
Copy link
Collaborator

Can one of the admins verify this patch?

@devOpsHazelcast
Copy link
Collaborator

devOpsHazelcast commented Oct 10, 2020

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@kwart kwart left a comment

Choose a reason for hiding this comment

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

LGTM.
Thanks for the fix, @ndeevy.
Could you rebase it to the latest master, please, to resolve the conflict?

@kwart kwart added this to the 4.1 milestone Oct 13, 2020
@kwart
Copy link
Member

kwart commented Oct 14, 2020

It seems two unrelated commits leaked to this PR. Could you remove them from it?

I usually use the following approach when rebasing:

# update local copies of remote repositories
git fetch --all

# rebase to the latest version of the master branch (upstream points to the hazelcast/hazelcast repository in my case)
# during the interactive (-i) rebase you can remove the 2 unrelated commits
git rebase -i upstream/master

## if there is a conflict which needs manual resolution, I use the following:
# git mergetool
# git rebase --continue

@ndeevy
Copy link
Author

ndeevy commented Oct 14, 2020

Hi @kwart , yes, not sure how that happened sorry. I'll try to fix it.

@ndeevy
Copy link
Author

ndeevy commented Oct 15, 2020

Hi @kwart The PR just has my one original commit now. Thanks for your help.

@kwart
Copy link
Member

kwart commented Oct 15, 2020

It's better now. I'm sorry to say it, but it has again a conflict in ClientConfigXmlGenerator. I guess it's related to my change where I moved XML formatting code to the StringUtil class. Sorry for that, @ndeevy . Could you rebase once more?

image

@ndeevy
Copy link
Author

ndeevy commented Oct 18, 2020

Thanks @kwart :-] conflict should be gone now.

Copy link
Member

@kwart kwart left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kwart kwart merged commit e6997dd into hazelcast:master Oct 19, 2020
@kwart
Copy link
Member

kwart commented Oct 19, 2020

Congratulations, @ndeevy, the PR is merged. Thank you for your contribution!

@ndeevy
Copy link
Author

ndeevy commented Oct 19, 2020

Congratulations, @ndeevy, the PR is merged. Thank you for your contribution!

Thanks @kwart And thanks for your assistance too!

@hazelcast hazelcast deleted a comment from kwart Oct 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace "should" with "have to" in exception messages
5 participants