Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v14: Align Deploy interfaces, remove obsolete methods and default interface implementations #15965

Merged
merged 7 commits into from
Apr 10, 2024

Conversation

ronaldbarendse
Copy link
Contributor

This cleans up some of the Deploy interfaces within the CMS:

  • Align the IDataTypeConfigurationConnector.FromArtifact(...) return type with IDataType.ConfigurationData (following the data type refactor done in PR Datatype configuration refactor #13605);
  • Align the ArtifactBase<>.Alias property nullability with the IArtifact interface;
  • Ensure the ArtifactBase<>.Dependencies collection is lazily set when empty (only allocating when being iterated);
  • Remove ShouldSerializeChecksum(), since that was specific to the Newtonsoft.Json implementation (to avoid serializing the Checksum property without having to take a reference to add JsonIgnore attribute);
  • Remove obsolete methods and default interface implementations.

src/Umbraco.Core/Deploy/ArtifactBase.cs Outdated Show resolved Hide resolved
@bergmania
Copy link
Member

IMO should we look into a long term plan, about how to get these files out of the CMS. It looks like there is a failing unit test now.

@ronaldbarendse
Copy link
Contributor Author

IMO should we look into a long term plan, about how to get these files out of the CMS. It looks like there is a failing unit test now.

I've updated the test to include the Checksum property in the JSON output, as it is still using Newtonsoft.Json and removing the ShouldSerializeChecksum() method causes it to include the property again (which is actually the correct expectation, as it aligns with STJ).

I'm a bit split about removing these Deploy specific interfaces from the CMS though:

  • It does allow developers to add deployment support without having to take a dependency on Deploy (like Contentment does, see Notes/RenderMacro - added Deploy Connectors leekelleher/umbraco-contentment#225). This also removes the need for users to install additional packages to otherwise add this support. However, the provided interfaces only allow basic support for property values (IValueConnector) and data type configuration (IDataTypeConfigurationConnector), so you still need to install additional packages for adding support for e.g. custom entities (like Forms and Commerce, as you need to register your IServiceConnector with Deploy).
  • These interfaces can only be changed in major CMS versions (even in a backwards-compatible way, as Deploy currently supports all CMS versions of a specific major), making the development workflow a bit cumbersome and slower. Moving them to Deploy would make this easier, but at the expense of providing the mentioned built-in deployment support. Deploy 13 does utilize the CMS entity reference tracking (IDataValueReference[Factory] implementations), which allows very basic deployment support for adding dependencies though. We don't have something similar for data type configurations, which would also require a bit more context to know which dependencies should be processed first (the 'ordering' dependencies) and whether they only have to exist or need to exactly match between environments.

@bergmania bergmania merged commit 4c3c3cd into v14/dev Apr 10, 2024
15 of 16 checks passed
@bergmania bergmania deleted the v14/feature/align-deploy-interfaces branch April 10, 2024 08:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants