Enable culture analyzer and fix issues #3678
Conversation
Evangelink
force-pushed
the
culture-info-analyzers
branch
2 times, most recently
from
531a497 to
ca7d537
Compare
Evangelink
changed the title
Evangelink
force-pushed
the
culture-info-analyzers
branch
from
a317579 to
a88ce73
Compare
Evangelink
force-pushed
the
culture-info-analyzers
branch
from
a88ce73 to
43f7e56
Compare
Evangelink
changed the title
Evangelink
changed the title
| @@ -478,7 +478,7 @@ private void LoadAndInitialize(DataCollectorSettings dataCollectorSettings, stri | |||
|
|
|||
| if (!IsUriValid(dataCollectorUri) && !TryGetUriFromFriendlyName(dataCollectorSettings.FriendlyName, out dataCollectorUri)) | |||
| { | |||
| LogWarning(string.Format(CultureInfo.CurrentUICulture, Resources.Resources.UnableToFetchUriString, dataCollectorSettings.FriendlyName)); | |||
| LogWarning(string.Format(CultureInfo.CurrentCulture, Resources.Resources.UnableToFetchUriString, dataCollectorSettings.FriendlyName)); | |||
There was a problem hiding this comment.
What is the reason for changing this from ui culture to current culture?
There was a problem hiding this comment.
The analyzer is complaining about it :)
Rule documentation says:
CultureInfo.CurrentUICulture is used only to retrieve localized resources by using an instance of the System.Resources.ResourceManager class.
|
Imho in testhost we are setting current_UI culuture for the process and so I guess not using currentUIculture for formatting changes how messages are written to the screen. Did you do any tests to make sure that is not the case? |
|
@nohwnd I haven't done any special test, I was expecting our test suite to catch translation issues. I will try to see if I can run something manually to confirm things are ok. |
|
I doubt that will happen, all the tests are in english, and all the computers are running in english. |
|
@nohwnd I am too used to roslyn where one CI is running in foreign language to ensure translation are tested. |
Evangelink
force-pushed
the
culture-info-analyzers
branch
2 times, most recently
from
4ad9320 to
ef0d664
Compare
|
I have run a couple of manual tests using French culture and I haven't seen any problem. Do you know any special test we can run? |
Evangelink
force-pushed
the
culture-info-analyzers
branch
from
ef0d664 to
89e9287
Compare
Evangelink
force-pushed
the
culture-info-analyzers
branch
from
89e9287 to
b0e6475
Compare
Does that mean you are able to run on non-english system, and revert the output back to english usnig that env variable and vice versa? That is imho the purpose of that env variable. |
|
@nohwnd I confirm that fr-fr is working well on a US machine, and en-us on an FR machine works too. |
I have tried to group changes per project, set of projects.