U4-11428 - CodeAnnotations classes have inconsistent constructor parameter order [v8]

Created by Andy Thompson 08 Jun 2018, 23:23:12 Updated by Sebastiaan Janssen 19 Jul 2018, 07:14:42

Tags: PR

Subtask of: U4-11279

== Branch ==

temp8

== Problem ==

Attribute classes defined in the Umbraco.Core.CodeAnnotations namespace have inconsistent constructor parameter order. By this I mean that sometimes a parameter from the single parameter constructor will appear either as the first or second parameter in a multiple parameter constructor.

The classes with multiple constructors are below, with their parameters.

||Attribute class||Constructor One Parameters||Constructor Two Parameters||Notes|| |ActionMetadataAttribute|string category|string category, string name| | |UmbracoExperimentalFeatureAttribute|string description|string trackerUrl, string description|Does not appear to be used in v8| |UmbracoObjectTypeAttribute|string objectId|string objectId, Type modelType| | |UmbracoProposedPublicAttribute|string description|string trackerUrl, string description|Does not appear to be used in v8| |UmbracoWillObsoleteAttribute|string description|string trackerUrl, string description|Only the single parameter version appears to be used in v8|

== Solution ==

The constructor parameters should stay in the same position in each overload. Luckily the "incorrect" attributes either are not used or only have a single parameter version being used. If they are to be normalised it should be done before the attributes are used too often.

Comments

Andy Thompson 16 Jul 2018, 21:27:30

PR submitted as no-one objected to the proposed changes :-)

https://github.com/umbraco/Umbraco-CMS/pull/2781


Andy Thompson 17 Jul 2018, 15:07:17

Hi @ssougnez

The attributes classes I've changed do not exist in the v7 codebase. This PR and ticket are for v8 only. I've changed the affected version back to v8.


Sébastien Sougnez 17 Jul 2018, 15:22:03

Hi @Andy

Sorry, the change was not intentional... I wanted to filter issues by version but I actually changed the affected version of this one. Didn’t know I could update issue created by someone else... my bad :-$


Andy Thompson 17 Jul 2018, 15:27:50

@ssougnez haha ;-) no probs.


Priority: Minor

Type: Bug

State: Fixed

Assignee:

Difficulty: Very Easy

Category: Architecture

Backwards Compatible: True

Fix Submitted: Pull request

Affected versions: 8.0.0

Due in version: 8.0.0

Sprint:

Story Points:

Cycle: