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

732: Fix incorrect default in for_export() function for filters. #738

Closed
wants to merge 1 commit into
base: develop
from

Conversation

Projects
None yet
2 participants
@toolstack
Contributor

toolstack commented Jun 21, 2017

for_translation() already handles the default so passing in a null is fine so don't bother setting the default in for_export().

Resolves #732.

Fix incorrect default in for_export() function for filters.
for_translation() already handles the default so passing in a null is fine so don't bother setting the default in for_export().

@toolstack toolstack requested a review from ocean90 Jun 21, 2017

@toolstack toolstack self-assigned this Jun 21, 2017

@toolstack toolstack added this to the 2.4 milestone Jun 21, 2017

@@ -299,7 +299,7 @@ public function set_fields( $db_object ) {
}
public function for_export( $project, $translation_set, $filters = null ) {

This comment has been minimized.

@ocean90

ocean90 Jun 21, 2017

Member

The default of $filters should probably be changed to an array now since that's the expected type by for_translation().

@ocean90

ocean90 Jun 21, 2017

Member

The default of $filters should probably be changed to an array now since that's the expected type by for_translation().

This comment has been minimized.

@toolstack

toolstack Jun 21, 2017

Contributor

Makes sense I'll update it later.

I think there's a problem though with my solution so it may need an update, I'll verify it before I merge.

@toolstack

toolstack Jun 21, 2017

Contributor

Makes sense I'll update it later.

I think there's a problem though with my solution so it may need an update, I'll verify it before I merge.

@toolstack

This comment has been minimized.

Show comment
Hide comment
@toolstack

toolstack Jun 22, 2017

Contributor

The PR simply inverts the problem, causing the "all current" export to break, closing this PR and opening a new one to resolve the issue.

Contributor

toolstack commented Jun 22, 2017

The PR simply inverts the problem, causing the "all current" export to break, closing this PR and opening a new one to resolve the issue.

@toolstack toolstack closed this Jun 22, 2017

@toolstack toolstack deleted the 732-fix-export-matching-filter branch Jun 22, 2017

@toolstack toolstack modified the milestones: 3.0, 2.4 Aug 15, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment