A bunch of random work items/balloon payment on technical debt - thoughts?

[dodexahedron]

So...

There are a ton of random work items that I keep wanting to address every time I come across them. However, I mostly don't do it, because they tend to only be incidental to the scope of the issue I'm working on, and potentially through several layers of indirection at that.

But I keep coming across them and have a bunch of stashes in my local repositories that are just feeling all sad because they aren't getting any love.

Many (like...probably 80% or more) are also either compiler warnings or other points alerted by analyzers from VS, R#, etc., so addressing them will reduce a lot of noise, at the very least.

These changes fall almost entirely into two non-exclusive categories:

• Code quality/syntax/best practices
• Performance

The vast majority are not behavior-changing, in terms of the rendered output, and I usually try to make them 100% drop-in changes when possible. But, they can occasionally represent minor source-breaking changes, typically internally but occasionally also in the public API surface. I of course resolve those and ensure they continue to work as expected, when that situation arises.

A lot of them also have a common impact on multiple other changes I'm working on or planning to work on, and I'd rather get them done before starting or continuing on those, so that I can have the benefit of that code in each without making identical and therefore conflicting changes in each.

What are your opinions on how best to handle that stuff, in terms of issues/PRs?

My preference is to just make one cleanup/code quality issue, do an arbitrary amount of that work in it, and then make a PR for that when I feel like it's good enough for that round. Further work would then simply just be left for future issues, potentially of the same type.

That provides the benefits to everyone more rapidly, helps avoid conflicts, and allows the work to actually get done, rather than meticulously documenting every single issue, many of which are one-liners.

I make micro-commits anyway, and this kind of change set lends itself very well to that, so the history is fully visible in the repo log.

So... Whaddya think? Do it like that? Some other way?

[dodexahedron]

Some examples of changes that aren't compiler warnings that fall under this general umbrella are:

• Use of switch statements instead of large if/else constructs, when possible
• Declaring interfaces that some types already support with their current implementations, so they can be accessed/used via those interfaces by consumers (Such as the IEqualityOperators<T1,T2> interface I declared on a couple of types that have those interface members already implemented)
• Implementation of additional interfaces that make sense on certain types (like how I put the INofityCollectionChanged and IDictionary<TKey,TValue> on ColorScheme)
• Naming and other similar changes that either clarify a piece of code or bring it more into compliance with project guidelines and conventions
• Use of BCL types instead of custom types, where possible, especially in cases where our code was literally copied from the BCL anyway (Color, Point, Rect[angle], Size, etc)
• Use of managed methods instead of PInvokes for functionality that is available in .net, either because it has been added since original implementation (most cases) or even if someone just didn't happen to find it in .net first.
  • These are always benchmarked to ensure there's not some sort of negative performance impact.
• Reference passing for value types, when possible
• Performance enhancements such as use of Spans in implementations, more efficient string handling, reducing unnecessary memory copies, avoiding boxing of value types, etc
• Language feature usage like collection expressions
• Spelling and grammar fixes or improvements
• Clarification, correction, etc of some XmlDoc comments
• Nullability annotations and cleaner handling of existing null checks (beyond the simple replace of == to is from my recent PR)
• Adding comments (using various tags that will make them show up in the ReSharper TODO explorer) in the code where I find other issues that are not appropriate to be directly addressed in the scope of this work, like:
  • Concurrency issues
  • Potential bugs/unintended behavior
  • Comments I may have for future work or improvements
  • Questions I have about a piece of code or its intent
  • Requests for clarity
  • Questions and suggestions related to unconventional or otherwise strange code/constructs/algorithms/behaviors
  • Anything else that is out of scope or just being noted for the future

[dodexahedron]

Closing this, as work is proceeding as described and some has already been merged. 👍