Skip to content

Code styling issues #433

Closed
Closed
@macsux

Description

@macsux

With the recent introduction of code style rules, I wanted to bring up a few issues I hope we can address:

  1. Some types of checks are not reformatted using dotnet format. This causes a significant loss in productivity trying to fix up formatting issues. Examples of where auto formatted doesn't work is a requirement for extract lines after braces. If there's another tool that can "auto-format", I think it should be put into README.md as guidance
  2. I disagree with SA1503: Braces should not be ommited for if statements. A lot of validation checks such as if (type == null) throw new ArgumentNullException(nameof(type)); turn into 4 lines of code
            if (type == null)
            {
                throw new ArgumentNullException(nameof(type));
            }

which only detracts from the readability of code. I propose this rule is removed
3. There are rules in there right now to validate things like namespace naming convention which k8s is failing
4. Legitimate warnings are being drowned in the formatting "noise" on build atm, some of which we won't be able to address (like namespace naming).
5. I'm also noticing a VERY significant degradation of IDE performance with the latest style changes, as I suspect there's a ton of background threads trying to run Roslyn analyzers.
6. In some cases, the code actually becomes less efficient. Example: JObject.Contains is defined explicitly in the newer version but older the one that targets 4.5.2. The current rule makes you cast it to IDictionary as you're not supposed to access it implicitly, meaning newer targets are using less optimized code path.

I think there needs to be a balance struck on which rules to enforce, as previous experience with projects that enforced very strict linting rules without the ability to automatically reformat the code has discouraged contributions as it turned into a major chore.

Metadata

Metadata

Assignees

No one assigned

    Labels

    lifecycle/rottenDenotes an issue or PR that has aged beyond stale and will be auto-closed.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions