Skip to content

Enable Culture Code Quality Rules#20957

Merged
Mailaender merged 5 commits into
OpenRA:bleedfrom
RoosterDragon:style-ca-culture
Aug 7, 2023
Merged

Enable Culture Code Quality Rules#20957
Mailaender merged 5 commits into
OpenRA:bleedfrom
RoosterDragon:style-ca-culture

Conversation

@RoosterDragon
Copy link
Copy Markdown
Member

Following on from #20814.

Enables a batch of rules related to culture/format/comparison correctness. Although these don't detect all patterns, they enforce the the culture, format or comparison of string related parsing, ToString or comparsions. This is particuarly helpful since if not specified culture and format defaults to the current culture - i.e. the culture of the user. This tends to break most code which would prefer to parse and format in the invariant culture and falls over if the user's culture doesn't line up as expected (i.e. if you dare to use anything other than en-US conventions).

I have specified oridinal/invariant comparsions for most parsing and formatting related code, and for user facing inputs or display have used their culture. The culture specified in some areas has changed compared to before. Please review and see if you agree with the chosen settings.

Comment thread OpenRA.Mods.Common/Widgets/Logic/Ingame/ObserverStatsLogic.cs Outdated
Comment thread OpenRA.Mods.Common/Widgets/Logic/Editor/ActorEditLogic.cs
@RoosterDragon RoosterDragon force-pushed the style-ca-culture branch 2 times, most recently from db4fb4c to ebaa68a Compare July 9, 2023 12:22
PunkPun
PunkPun previously approved these changes Jul 9, 2023
Copy link
Copy Markdown
Member

@PunkPun PunkPun left a comment

Choose a reason for hiding this comment

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

LGTM,

My local culture is american english so I might have missed something

Comment thread OpenRA.Mods.Common/Widgets/Logic/Settings/HotkeysSettingsLogic.cs
@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Jul 10, 2023

I'd advise to not merge this before the prep branch is split off

@Mailaender
Copy link
Copy Markdown
Member

I think these are pretty save. They only prevent common errors we made in the past with non US locales.

@RoosterDragon RoosterDragon force-pushed the style-ca-culture branch 2 times, most recently from 23dd22f to e836f9a Compare July 20, 2023 19:41
@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Jul 30, 2023

needs a rebase

@Mailaender
Copy link
Copy Markdown
Member

Needs a rebase.

@PunkPun
Copy link
Copy Markdown
Member

PunkPun commented Aug 7, 2023

rebased

@Mailaender Mailaender merged commit 388222c into OpenRA:bleed Aug 7, 2023
@Mailaender
Copy link
Copy Markdown
Member

Changelog

@RoosterDragon RoosterDragon deleted the style-ca-culture branch August 8, 2023 16:46
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.

4 participants