CommentFileSizeAuthor
#11 Merge-two-functions-2450833-5-7.patch5.03 KBajalan065
#9 Merge-two-functions-2450833-5-7.patch5.13 KBajalan065
#5 Merge-two-functions-2450833-5-7.patch5.27 KBajalan065
#2 2450833.patch5.16 KBnaveenvalecha
#2 2450833.patch5.16 KBnaveenvalecha
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented

I could not understand the need to merge these two functionalities.
Both perform different tasks, so why merge them?

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented
Status: Active » Needs review
FileSize
2450833.patch5.16 KB
2450833.patch5.16 KB

both have similar functionality, the common function name should be uc_wishlist_save_wishlist
Here we go.

The last submitted patch, 2: 2450833.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 2: 2450833.patch, failed testing.

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented
Status: Needs work » Needs review
FileSize
Merge-two-functions-2450833-5-7.patch5.27 KB
naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented

interdiff ? https://www.drupal.org/documentation/git/interdiff

Status: Needs review » Needs work

The last submitted patch, 5: Merge-two-functions-2450833-5-7.patch, failed testing.

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented

@naveenvalecha, I tried to reroll the patch, as the current patch does not applies cleanly.

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented
Status: Needs work » Needs review
FileSize
Merge-two-functions-2450833-5-7.patch5.13 KB

Status: Needs review » Needs work

The last submitted patch, 9: Merge-two-functions-2450833-5-7.patch, failed testing.

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented
FileSize
Merge-two-functions-2450833-5-7.patch5.03 KB

Resolved the path conflicts which was occurring during application of patch

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: Merge-two-functions-2450833-5-7.patch, failed testing.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented

@ajalan065, the drupal ci is failing b/c there are not existing tests in repo. please do manual testing

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented

@naveenvalecha, I was stuck due to some work.
I will do the manual testing as soon as possible.

ajalan065’s picture

ajalan065 CreditAttribution: ajalan065 as a volunteer and at Google Summer of Code commented

@naveenvalecha, the patch works as expected.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented
Status: Needs work » Fixed
Parent issue: » #2386985: Module Release 7.x-1.3

Thanks Arpit!
Committed and pushed to 7.x-1.x will be included in next release.

naveenvalecha’s picture

naveenvalecha
He/Him
Primary language Hindi
CreditAttribution: naveenvalecha as a volunteer and at Acquia for Acquia commented

This would be the next one to get this in https://www.drupal.org/node/2397457

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.