Skip to content

Conversation

@dyurchenko-epam
Copy link
Contributor

@coveralls
Copy link

coveralls commented Nov 28, 2019

Coverage Status

Coverage increased (+0.001%) to 99.199% when pulling 9feee4b on bugfix/HW-54154-request-example-for-create-user-on-php-setDateOfBirth-method-is-not-correct into 4f27456 on master.

public function setDateOfBirth($dateOfBirth = null)
{
$stringToDate = $dateOfBirth === null ? null : new \DateTime($dateOfBirth);
$this->dateOfBirth = $stringToDate == null ? null : $stringToDate->format('Y-m-d');
Copy link
Contributor

Choose a reason for hiding this comment

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

As this date manipulation is done in several places, I suggest to make a common util method and use it in different places

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thank you.


class StringToDataConverter
{
static function convertStringToDate($string, $format)
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we cover this method with tests?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@gmeyer-hw gmeyer-hw left a comment

Choose a reason for hiding this comment

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

I read the requirement and I noticed the doc-portal doesn't contain valid code snippet for the Python, Java and PHP SDK. I added more info on the ticket.

@dyurchenko-epam
Copy link
Contributor Author

It's not necessary anymore because there is an another pr for solving this problem https://github.com/hyperwallet/program-portal/pull/215

@dyurchenko-epam dyurchenko-epam deleted the bugfix/HW-54154-request-example-for-create-user-on-php-setDateOfBirth-method-is-not-correct branch December 6, 2019 15:11
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.

7 participants