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.

Sign up for GitHub

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

Jump to bottom

[REF] Extract duplicate handling code #19103

Merged
merged 1 commit into from
Dec 4, 2020
Merged

[REF] Extract duplicate handling code #19103

merged 1 commit into from
Dec 4, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

Overview

[REF] Extract duplicate handling code

Before

Code in long function

After

Code in own function

Technical Details

This winds up being a very strange process - however the inital goal seems to be
to unravel error handling to appropriate exception throwing & catching
& a bit of back & forth helps us isolate the code places

Comments

@civibot
Copy link

civibot bot commented Dec 3, 2020

(Standard links)

@civibot civibot bot added the master label Dec 3, 2020
@seamuslee001
Copy link
Contributor

@eileenmcnaughton style issues https://test.civicrm.org/job/CiviCRM-Core-PR/38192/checkstyleResult/new/

@eileenmcnaughton
Copy link
Contributor Author

looks like that fixed it

@seamuslee001
Copy link
Contributor

@eileenmcnaughton test fails relate

CRM_Contact_Import_Parser_ContactTest.testImportParserWithUpdateWithoutExternalIdentifier
CRM_Contact_Import_Parser_ContactTest.testImportDeceased
CRM_Contact_Import_Parser_ContactTest.testImportFill
CRM_Contact_Import_Parser_ContactTest.testImportAmbiguousStateCountry

@eileenmcnaughton
Copy link
Contributor Author

Hmm looks like I deleted a word during reformatting & hence broke it - I think it should be right now

@seamuslee001
Copy link
Contributor

@eileenmcnaughton same 4 tests fail still

@eileenmcnaughton
[REF] Extract duplicate handling code
697f7b5 
This winds up being a very strange process - however the goal seems to be
to unravel error handling to appropriate exception throwing & catching
& a bit of back & forth helps us isolate the code places
@eileenmcnaughton
Copy link
Contributor Author

@seamuslee001 not quite ... we are back to the same 4 failing. That's an achievement :-)

Anyway my type was too strict so should be OK next time

@eileenmcnaughton
Copy link
Contributor Author

test this please

@eileenmcnaughton
Copy link
Contributor Author

fail seems odd

Stacktrace
CRM_Core_BAO_CustomFieldTest::testFileDisplayValueNoDescription
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-' \n
+' \n
\n
'

/home/jenkins/bknix-dfl/build/core-19103-33p9p/web/sites/all/modules/civicrm/tests/phpunit/CRM/Core/BAO/CustomFieldTest.php:937
/home/jenkins/bknix-dfl/build/core-19103-33p9p/web/sites/all/modules/civicrm/tests/phpunit/CiviTest/CiviUnitTestCase.php:226
/home/jenkins/bknix-dfl/extern/phpunit7/phpunit7.phar:615

@seamuslee001
Copy link
Contributor

Looks good now

@seamuslee001 seamuslee001 merged commit 4e13ba2 into civicrm:master Dec 4, 2020
@seamuslee001 seamuslee001 deleted the import_extract branch December 4, 2020 19:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
master merge on pass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@eileenmcnaughton @seamuslee001