Skip to content

Fix #6825 by reducing the changed files to those only related to the issue#7916

Merged
sebastienros merged 3 commits intoOrchardCMS:1.10.xfrom
HengzheLi:#6825
Jan 4, 2018
Merged

Fix #6825 by reducing the changed files to those only related to the issue#7916
sebastienros merged 3 commits intoOrchardCMS:1.10.xfrom
HengzheLi:#6825

Conversation

@HengzheLi
Copy link
Copy Markdown
Contributor

#6825 fixed #3275 by @jersiovic, but it was prevented from merging as it changed too many files.

This PR, I branch it from master, only apply the commit 106ff07 and bd531cf from jersiovic in #6825 of which were key commits for fixing #3275 and there are any other changes.

PS: hope this PR does not offense your contributions @jersiovic as it reuse your works. If it does, I will close it.

@dnfclas
Copy link
Copy Markdown

dnfclas commented Dec 16, 2017

CLA assistant check
All CLA requirements met.

@jersiovic
Copy link
Copy Markdown
Contributor

@li0803 no problem, thank you for submitting the PR

Comment thread ClickToBuild.cmd
@@ -1,15 +1,26 @@
for /f "usebackq tokens=*" %%i in (`lib\vswhere\vswhere -latest -version "[15.0,16.0)" -requires Microsoft.Component.MSBuild -property installationPath`) do (
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Changes in this file are really needed? It looks that are not really related with thi topic of the PR

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well, the topic of the PR in reality is very open

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I made this branch from master. And this change is a head commit to 1.10.x by sebastienros on master branch.
And master is a stable branch. So I think I'd better submit this PR to another one.

<Compile Include="Models\NavigationQueryPartRecord.cs" />
<Compile Include="Models\QueryPart.cs" />
<Compile Include="Providers\Builders\MemberBindingsStep.cs" />
<Compile Include="Providers\Excutors\MemberBindingsStep.cs" />
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Typo Excutors should be Executors

using Orchard.Recipes.Services;

namespace Orchard.Projections.Providers.Builders
{
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Better use curly braces on the same line following style used in Orchard for consistency for all the files in this PR

@sebastienros
Copy link
Copy Markdown
Member

Thanks, please apply the changes requested by @jersiovic

@HengzheLi
Copy link
Copy Markdown
Contributor Author

apply jersiovic 2) and 3) review suggestion

@sebastienros sebastienros merged commit 9839cec into OrchardCMS:1.10.x Jan 4, 2018
@HengzheLi HengzheLi deleted the #6825 branch April 9, 2018 03:26
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