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

Fix coding style issues #899 in tests/ #926

Closed
wants to merge 6 commits into from
Closed

Fix coding style issues #899 in tests/ #926

wants to merge 6 commits into from

Conversation

jongfeli
Copy link
Contributor

I summarized the ones that are left in tests/. I am more then happy to fix those that can be fixed or improved but not without help. I would appreciate if someone could mark the ones that can be fixed or improved and if so explain how to do this, when possible.

And for the ones that can not be fixed but should be sniffed should we use @codingStandardsIgnoreStart and @codingStandardsIgnoreEnd tags like suggested in #899 but how? I had look in [ConjunctionConditionBuilder.php] but I am not sure if this is correct:

Now:

            if ( $subCondition instanceof FalseCondition ) {
                return new FalseCondition();
            } elseif ( $subCondition instanceof TrueCondition ) {
                // ignore true conditions in a conjunction
            } elseif ( $subCondition instanceof WhereCondition ) {
                $subConditionElements->condition .= $subCondition->condition;
            } elseif ( $subCondition instanceof FilterCondition ) {

Becomes:

            if ( $subCondition instanceof FalseCondition ) {
                return new FalseCondition();
            } // @codingStandardsIgnoreStart
            elseif ( $subCondition instanceof TrueCondition ) {
                // ignore true conditions in a conjunction
            } // @codingStandardsIgnoreEnd
            elseif ( $subCondition instanceof WhereCondition ) {
                $subConditionElements->condition .= $subCondition->condition;
            } elseif ( $subCondition instanceof FilterCondition ) {

@JeroenDeDauw
Copy link
Member

The example change you posted looks good to me

@JeroenDeDauw
Copy link
Member

I summarized the ones that are left in tests/. I am more then happy to fix those that can be fixed or improved but not without help. I would appreciate if someone could mark the ones that can be fixed or improved and if so explain how to do this, when possible.

Cool! I'll have a look and leave inline comments here jongfeli/Code-violations-SemanticMediaWiki@f358e97

@JeroenDeDauw
Copy link
Member

Protip: make your commits on a branch (that is not master) and then push the branch to your GitHub remote

@JeroenDeDauw
Copy link
Member

I've submitted your commits plus a merge conflict fix from me in a new PR: #931

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.

None yet

2 participants