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

Replace DigitalAssets with Resource Manager in templates and remove DigitalAssets module #5194

Merged

Conversation

david-poindexter
Copy link
Contributor

@david-poindexter david-poindexter commented Jul 20, 2022

Summary

Resolves #4895

  • Updated Blank Website template to replace Digital Assets module with Resource Manager
  • Updated Default Website template to replace Digital Assets module with Resource Manager
  • Updated admin template (though this is not used in the UI) to replace Digital Assets module with Resource Manager
  • Removed DigitalAssets module

@david-poindexter david-poindexter changed the title Issue 4895 Replace DigitalAssets with Resource Manager in templates and remove DigitalAssets module Jul 20, 2022
@mitchelsellers
Copy link
Contributor

Question - Should digital assets live on in a separate repo? (I don't believe so, but just asking)

@valadas
Copy link
Contributor

valadas commented Jul 20, 2022

I don't think so either, we should not maintain a Telerik-based solution IMO and if someone really really wants to, they can grab it from the git history.

Copy link
Contributor

@valadas valadas left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks guys!

@valadas valadas added this to the 9.11.0 milestone Jul 21, 2022
@valadas
Copy link
Contributor

valadas commented Jul 21, 2022

Strange the build failed creating nuget packages but the artifact is fine, I'll try to re-fire the build

@valadas
Copy link
Contributor

valadas commented Jul 21, 2022

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@david-poindexter
Copy link
Contributor Author

@david-poindexter david-poindexter marked this pull request as ready for review July 21, 2022 20:58
@valadas
Copy link
Contributor

valadas commented Jul 22, 2022

I believe the problem is in the bundle:

https://github.com/dnnsoftware/Dnn.Platform/blob/develop/Build/Tools/NuGet/DotNetNuke.Bundle.nuspec#L33-L34

That makes sense, why do include the DAM in there? I think we could simply remove it from the nuspec file right ?

@david-poindexter
Copy link
Contributor Author

david-poindexter commented Jul 22, 2022

Yeah, I think so. I'll test that now.

@david-poindexter
Copy link
Contributor Author

Okay, this PR is ready for a second review. CC @dnnsoftware/approvers

Copy link
Contributor

@mitchelsellers mitchelsellers left a comment

Choose a reason for hiding this comment

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

This looks good, thank you!

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.

None yet

4 participants