Fixing grammer of & improving CA1836 messages.

[BrunoJuchli]

Improving the message of PreferIsEmptyOverCountMessage (CA1836):

old : Prefer 'IsEmpty' over 'Count' to determine whether the object contains or not any items
new: Prefer 'IsEmpty' over 'Count' to determine whether the object is empty

Also slightly improved the german & french translations.

[Evangelink]

Minor suggestion, otherwise LGTM.

Tagging @mavasani

[Evangelink]

@BrunoJuchli Sorry I missed it, the message string should not end with a period.

[mavasani]

Yes, this will cause build failure.

[mavasani]

Also tagging @jozkee for review.

[BrunoJuchli]

Thank you. I've removed the trailing period in the Message.

@mavasani
I've also done so in the French and in the German translations (even though it did have a trailing period before). I guess that's ok (but the build is not checking translations for correctness in that regard) -- right?
I haven't checked/fixed other languages. I could but then I believe it would probably be best to create a separate PR and remove all the trailing periods of all the translated messages - do you concur?

[Evangelink]

@BrunoJuchli You are right, the analyzer only check the English message, title and description entries. I was thinking about checking the other languages but that would probably require to discuss with native speakers to see what they think. Basically we wrote this analyzer to ensure that there is a consistent style in how messages are shown to the user (at least partially).

[mavasani]

LGTM once you remove the period at the end of the new message string and update all the xlf files accordingly.

[codecov]

Codecov Report

Merging #4210 (5f98c98) into master (fb97add) will increase coverage by 0.00%.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #4210   +/-   ##
=======================================
  Coverage   95.83%   95.84%           
=======================================
  Files        1177     1177           
  Lines      268158   268158           
  Branches    16192    16192           
=======================================
+ Hits       256997   257010   +13     
+ Misses       9084     9066   -18     
- Partials     2077     2082    +5     

[jozkee]

I think you need to mention two alternatives to properly use "whether".

Suggested change

	<value>Prefer 'IsEmpty' over 'Count' to determine whether the object is empty</value>
	<value>Prefer 'IsEmpty' over 'Count' to determine whether the object is empty or not</value>

Suggested change

	<value>Prefer 'IsEmpty' over 'Count' to determine whether the object is empty</value>
	<value>Prefer 'IsEmpty' over 'Count' to determine whether or not the object is empty</value>

We could even rephrase to not use the word

Suggested change

	<value>Prefer 'IsEmpty' over 'Count' to determine whether the object is empty</value>
	<value>Prefer 'IsEmpty' over 'Count' to determine if the object is empty</value>

[BrunoJuchli]

@jozkee
I'm not a native speaker so I looked it up.,The first two resources I found don't back that up
( https://english.stackexchange.com/a/48773/398754, http://www.youtube.com/watch?v=Klnroe1UBRs&t=1m23s <-- here making a specific example with whether and stating that "or not" is not necessary).
So I think it's rather a stylistic choice. Personally I would go for the shorter version unless there's ambiguity.
That would make me lean toward the if version (probably also most commonly understood by non native speakers; every programmer will know if introduces a condition). Though the guy in the youtube video said that whether specifies a binary condition and if goes well with multiple options.

Maybe we could have an odd number of native speakers vote on a version? :D

[mavasani]

Lets go with the changes and if required/based on feedback, we can change them in future.

[mavasani]

@BrunoJuchli Can you please rebuild locally to resolve conflicts?

[BrunoJuchli]

@mavasani I just rebased on current master and force pushed. Strangely this PR doesn't seem to pick up the change, though stackoverflow states multiple times force-pushing to a branch that has an open PR is getting updated.
Will wait until tomorrow to see if it sync's eventually, otherwise I'll go back to before-rebase version and merge in latest develop.

[mavasani]

Closing and re-opening PR to retrigger tests, as we checked in new CI validation step for generated files.

[mavasani]

LGTM

@buyaa-n Please feel free to review and merge if the changes look good to you. Thanks!