Umbraco-cms: Add Stylecop Analysers and verbose .editorconfig to the .NET Core solution.

[JimBobSquarePants]

Umbraco's codebase is extremely difficult to navigate even for experienced developers. The lack of code consistency and intellisense documentation leads to easy to avoid mistakes.

Porting the solution to .NET Core is a massive challenge, made much more difficult by this lack of consistency.

I propose that this is the perfect opportunity to introduce linting tools that will help enforce code consistency and quality for a new codebase that will be the building block for a generation of development.

Contrary to previously expressed opinion, introducing rules does not make it less easy to contribute. A strong set of rules aids with initial development, testing, documentation generation, as well as future development by reducing the number of bugs and increasing the readability of code.

Remember, code is read many more times than it is ever written.

I own a collection of open source repositories that deal with an incredibly complex space (2D graphics). For all that complexity there are comparatively few issues considering how many users there are. This is direct result of strong rules designed to help reduce mistakes in the codebase.

I have rules you can take that work well with separate rulesets for source and test repositories.

I implore you to take this request seriously. There are significant technical and operational cost savings that can be made which would beneficial to Umbraco as a company in addition to the open source libraries.

[drpeck]

I imagine the addition would create an initial commit with 10,000s of lines changes. If that makes it harder to keep the core version in-sync with V8 developments then perhaps identical rules need to be applied to both simultaneously.

[JimBobSquarePants]

Nothing is automatically applied.

[soreng]

Installing Stylecop in the https://github.com/umbraco/Umbraco-CMS/tree/netcore/netcore version of Umbraco.Core, generates 19000+ warnings currently. I imagine at least 98% of them can be fixed by just formatting the code

[JimBobSquarePants]

By formatting I'm assuming adding braces, intellisense docs, missing keywords, type declarations etc? Or do you mean whitespace?

One of the great things about analyzers is that you can fix issues in an entire document if need be. I'd take it slowly though. Introduce them into one project at a time and work through the solution.

[soreng]

@JimBobSquarePants yes - the ones I looked at was of the kind you mention.

[AndyButland]

I recall adding this for the first time to what I thought was one of my teams pristine code-bases and getting a nasty shock!... but once I'd mentally recovered from the offence caused - think I had to uninstall it and then put it back when I'd calmed down a bit - I've appreciated using it or similar tools from then on.

Agree it would be nice to have similar in Umbraco, for standardisation and consistency. It would also be something that's very open to the community to help with, potentially via lots of smaller PRs targeting a project or a set of rules, to help adhere to the agreed standards.

My only concern is the impact on merging in from V8 if there's a lot of change above and beyond the migration itself in the netcore branch (already this takes up a fair chunk of time each sprint). How much additional effort adding change like this would have needs to be considered. If it's not too much extra, then agree with the comment that this would be a good time to introduce such linting, as the plan obviously is that this will be the code base version that lives on long into the future.

[JimBobSquarePants]

To counter any point regarding increased porting any code I would state that by order of magnitude more time is saved during future work.

The old adage rings true here.

“Time spent is time saved”

In addition I would also say that as the .NET Core solution develops there are already going to be increased differences in the two codebases. Merging code from v8 will always become more difficult. (As an aside this is why most people do not attempt what Umbraco is doing and cut off feature development on one major version when working on a new one, especially with such a major paradigm shift).

[drpeck]

I'm inclined to agree, in contradiction to my previous point, so long as there is no effort to update the entire codebase (only as and when code is already being changed).

A big refactor would smell of V5.

[soreng]

(Re)Formatting code is not refactoring.

The preliminary impacts would be to ensure that code is being formatted the same - always add bracets, spacikg between parameters etc.

Later on naming rules could be enabled, That may require some mild refactoring.

On 5 Sep 2020, at 14.40, David Peck [e-mail] wrote:


I'm inclined to agree, in contradiction to my previous point, so long as there is no effort to update the entire codebase (only as and when code is already being changed).

A big refactor would smell of V5.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub, or unsubscribe.

[webmonger]

If the analysers are inlined between the framework codebase and changes are drawn in to the .net core codebase it should be more straightforward. That way changes made in framework will be far more straightforward to integrate than trying to make the core codebase one thing and the framework codebase another.

[drpeck]

(Re)Formatting code is not refactoring.

Fair point. My point was more specifically that a large number of lines of code would create a greater disparity between V8 and Project Unicore. PRs and merging in general would become more complex. It would be better done on a piecemeal basis IMO - I'm not clear if anyone is disagreeing with this.

[JimBobSquarePants]

Can we get a HQ response to this please?

[bergmania]

I talked with a couple of other people from HQ.
All of us think it is a good idea to introduce more rules to align the code base, as long as these are not introduced as severity = error.

Our main concern is how we can introduce the rules stepwise and ensure we don’t start with some rules most of us dislike.

[JimBobSquarePants]

Our main concern is how we can introduce the rules stepwise and ensure we don’t start with some rules most of us dislike.

You don't because then you end up with a weak set of rules that you gain little from due to developer laziness or resistance to positive change. The rules don't exist purely for the benefit of the current developer, they exist for the next developer and the next developer after that.

From a pure efficiency perspective incrementing the ruleset is grossly inefficient. If you gradually introduce rules then you end up repeatedly adjusting large numbers of files as the strictness of the rules increases.