dnnsoftware
[WIP] First code submission for MVC Pipeline
This PR is a first submission of the work of the MVC Pipeline team to bring the main project up to date with what has been developed in a forked repo.
Documentation
MVC Module Control Implementation MVC pipeline (routing / settings) Razor Module Development Guide
@bdukes @donker I believe we discussed that this is just a reset PR and initial review, as it is into their branch correct? High level review only?
@donker In looking at this at this moment the solution isn't building, would you be able to review this and resolve the issue?
@donker In looking at this at this moment the solution isn't building, would you be able to review this and resolve the issue?
@mitchelsellers I just try to build a fresh checkout of feature/mvc-pipeline-old without issuues (.\build.ps1).
Can you tell witch build you use or what kind of errors you get ?
@sachatrauwaen It is the AzureDevOps build that is failing, it looks like it is unit tests that are failing to execute.
The most recent run - https://github.com/dnnsoftware/Dnn.Platform/pull/6749/checks?check_run_id=51891737747
@bdukes @donker I believe we discussed that this is just a reset PR and initial review, as it is into their branch correct? High level review only?
Yes. This is what we discussed in an earlier meeting to have the first code submission a proper PR.
MVC Pipeline and CSP are 2 big ticket items and bringing both in the same PR makes putthing this in a reviewable state hard IMO. The goal of this PR was to have a baseline so that we can review smaller additions in an easier way.
There an separate PR for CSP on the develop branch.
But removing it from this PR needs to remove a lot of code. The idee is to replace the CSP project of this PR with the PR on develop when the PR is merged in develop.
Actually we heave a lot of sync issues between the differnt PR's. I don'tknow the best way. So your advice is welcome.
MVC Pipeline and CSP are 2 big ticket items and bringing both in the same PR makes putthing this in a reviewable state hard IMO. The goal of this PR was to have a baseline so that we can review smaller additions in an easier way.
There an separate PR for CSP on the develop branch.
But removing it from this PR needs to remove a lot of code. The idee is to replace the CSP project of this PR with the PR on develop when the PR is merged in develop.
Actually we heave a lot of sync issues between the differnt PR's. I don'tknow the best way. So your advice is welcome.
So if that is the case, we need to review/agree on the CSP one first before spending too much time reviewing this one...
MVC Pipeline and CSP are 2 big ticket items and bringing both in the same PR makes putthing this in a reviewable state hard IMO. The goal of this PR was to have a baseline so that we can review smaller additions in an easier way.
There an separate PR for CSP on the develop branch.
But removing it from this PR needs to remove a lot of code. The idee is to replace the CSP project of this PR with the PR on develop when the PR is merged in develop.
Actually we heave a lot of sync issues between the differnt PR's. I don'tknow the best way. So your advice is welcome.
So if that is the case, we need to review/agree on the CSP one first before spending too much time reviewing this one...
To simplify the review
- CSP is removed from this branch (i have just leaved some comments for later integration)
- remove module settings stuff, because actually out of scope of mvc pipeline
My suggestion is
before reviewing "First code submission for MVC Pipeline", merge this PRs without review PRs are inter dependend and replace a lot of code and are on this branch So after you will review the whole with a cleaner and more definitive view
- MVC pipeline - Module control — Type: Feature #6700
- MVC pipeline popups #6723
- MVC pipeline (routing / settings) #6721
- MVC pipeline - module actions #6725