Skip to content

Fixes: U4-2833 Packages using DataEditorSetting install alias.#205

Closed
leekelleher wants to merge 1 commit intoumbraco:6.2.0from
leekelleher:U4-2833
Closed

Fixes: U4-2833 Packages using DataEditorSetting install alias.#205
leekelleher wants to merge 1 commit intoumbraco:6.2.0from
leekelleher:U4-2833

Conversation

@leekelleher
Copy link
Member

Extends the package installer to check if a PreValue element has an Alias value.
Updates the DataTypeService to write the alias to the cmsDataTypePreValues database table.

Copy link
Member Author

Choose a reason for hiding this comment

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

I've been told that this is a breaking change, (apologies). I'll find an alternative solution and update the pull-request.

@sitereactor
Copy link
Contributor

I think using a Tuple to describe "something" in a collection should be an absolute last resort. IMO its not very obvious what is what alias/value or value/alias. Its easy to get wrong and its not mentioned in the code comment.

Did you exporer using a dictionary for alias / value?

@Shazwazza
Copy link
Contributor

Please just backport the changes to IDataTypeService from v7 - think stephen said he did that already?

@sitereactor
Copy link
Contributor

I can't figure out what you think is a breaking change? Did you already change the bit that is linked in the diff above?

@Shazwazza
Copy link
Contributor

PS... we are allowed to add methods to service interfaces, of course we cannot change or remove methods on these interfaces.

http://our.umbraco.org/documentation/Development-Guidelines/breaking-changes#Whatisabreakingchange?

@leekelleher
Copy link
Member Author

Thanks Morten/Shannon. I was lead to believe (at last week's UK hackathon) that adding a method to an interface was a breaking change - so if not, then great.

I'll fix up the Tuple issue and take a look at IDataTypeService changes in v7.

@Shazwazza
Copy link
Contributor

Hey lee, generally adding stuff to an interface would be but since developers cannot actually implement and use the interfaces defined here http://our.umbraco.org/documentation/Development-Guidelines/breaking-changes#Whatisabreakingchange this gives the core some flexibility when upgrading/fixing the data layer - otherwise we'd be completely stuck.

@leekelleher
Copy link
Member Author

Thanks again for the info @Shandem - understood.

I've taken a look at @zpqrtbnk's enhancements in the 7.0.0 branch - it looks solid to me. I'm wondering how much should (or could) be back-ported to 6.2.0? (Are you happy for me to do this? It wouldn't be until next week though)

Thanks, Lee.

@Shazwazza
Copy link
Contributor

DOH! disregard the thing i just wrote (have deleted now)!

Yes, we'd like to back port much of the service/repo layer to 6.2 but it will be quite manual due to the db changes made in v7.0. I've done a bit here and there already.

@sitereactor
Copy link
Contributor

The DataTypeService in v7 has a bunch CRUD in the service, which should be refactored to use a repository before its backported. At least IMO ;-)
Generally want to keep CRUD operations at the repository level.

@Shazwazza
Copy link
Contributor

yeah, just a few methods - would be easy to fix up

@nul800sebastiaan
Copy link
Member

Closing this one as it looks like we want to go another way with this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments