Skip to content

Conversation

@kvakvs
Copy link
Collaborator

@kvakvs kvakvs commented Jul 16, 2019

NOTE

This PR does not add, modify or remove features. This PR features code formatting, logging and quality of life changes to the developer experience.

Changes

This PR only includes files from CSUtil.Commons and TLM/Custom

For every file in TLM:

  • Untabify
  • Optimize imports (for FullDebug)
  • Format and wrap too long lines, or where StyleCop shows a warning
  • Apply minor Resharper suggestions and refactor
    • every var becomes explicit type (solves Enforce stating explicit types #436, debatable)
    • var Xxx used in out Xxx args, are inlined into out var Xxx (latest C#)
    • break deep nested if's where possible make the code more flat
    • extract huge code blocks into separate functions
  • Get rid of magic constants where possible
  • Mark redirected functions as [UsedImplicitly] to get rid of the warning
  • #if DEBUG if (condition) Log._Debug #endif changed to something shorter (solves Improve code readability by simplifying debug logging #349)

@kvakvs kvakvs added code cleanup Refactor code, remove old code, improve maintainability DO NOT MERGE YET Don't merge this PR, even if approved, until further notice labels Jul 16, 2019
@kvakvs kvakvs requested review from krzychu124 and originalfoo July 16, 2019 18:15
@kvakvs kvakvs self-assigned this Jul 16, 2019
@originalfoo originalfoo added this to the 11.0 milestone Jul 16, 2019
@kvakvs kvakvs added under-review A pull request has been created and is currently being reviewed and removed DO NOT MERGE YET Don't merge this PR, even if approved, until further notice labels Jul 19, 2019
Copy link
Member

@krzychu124 krzychu124 left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Tested in-game, no issues.

@krzychu124
Copy link
Member

Since we are reviewing this PR would be nice to change it's state to 'Ready for review' 😉

@kvakvs kvakvs marked this pull request as ready for review July 19, 2019 22:52
Copy link
Member

@originalfoo originalfoo left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@kvakvs kvakvs dismissed VictorPhilipp’s stale review July 23, 2019 14:17

I have addressed and responded to all notes except the multiline comments

@kvakvs kvakvs merged commit 94b27fe into CitiesSkylinesMods:master Jul 23, 2019
@originalfoo originalfoo added technical Tasks that need to be performed in order to improve quality and maintainability and removed under-review A pull request has been created and is currently being reviewed labels Jul 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

code cleanup Refactor code, remove old code, improve maintainability Harmony technical Tasks that need to be performed in order to improve quality and maintainability

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants