8000 Test default value declaration for the date type by greg0ire · Pull Request #2851 · doctrine/dbal · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Test default value declaration for the date type #2851

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

Merged
merged 1 commit into from
Sep 11, 2017

Conversation

greg0ire
Copy link
Member
@greg0ire greg0ire commented Sep 9, 2017

It was not covered, probably because it was hard to do something
generic. I choose to duplicate some tests instead, so that things stay
relatively simple.

It was not covered, probably because it was hard to do something
generic. I choose to duplicate some tests instead, so that things stay
relatively simple.
Copy link
Member
@Ocramius Ocramius left a comment

Choose a reason for hiding this comment

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

LGTM 🚢

@Ocramius Ocramius merged commit 6e3afd5 into doctrine:master Sep 11, 2017
@greg0ire greg0ire deleted the default_date_test branch September 11, 2017 06:15
Copy link
Member
@lcobucci lcobucci left a comment

Choose a reason for hiding this comment

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

Can you please open a PR with these small fixes?

'default' => $currentDateSql,
);

$this->assertEquals(
Copy link
Member

Choose a reason for hiding this comment

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

This should be self::assertSame()

Copy link
Member

Choose a reason for hiding this comment

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

This is applicable on the other files as well...

);

$this->assertEquals(
' DEFAULT '.$currentDateSql,
Copy link
Member

Choose a reason for hiding this comment

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

Missing spaces

Copy link
Member

Choose a reason for hiding this comment

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

This is applicable on the other files as well...

public function testGetDefaultValueDeclarationSQLForDateType()
{
$currentDateSql = $this->_platform->getCurrentDateSQL();
$field = array(
Copy link
Member

Choose a reason for hiding this comment

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

Please use short-array syntax

Copy link
Member

Choose a reason for hiding this comment

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

This is applicable on the other files as well...

@@ -563,6 +563,20 @@ public function testGetDefaultValueDeclarationSQLForIntegerTypes()
}
}

public function testGetDefaultValueDeclarationSQLForDateType()
Copy link
Member

Choose a reason for hiding this comment

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

testGetDefaultValueDeclarationSQLForDateType() : void

Copy link
Member

Choose a reason for hiding this comment

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

This is applicable on the other files as well...

@greg0ire
Copy link
Member Author

@lcobucci will do, but it would be great to have an automatic check for this

@lcobucci
Copy link
Member

@greg0ire we're adding most of the checks on PHPCS but @Ocramius didn't like my PR that fixed all CS violations, so we had to make allowed to fail in travis https://travis-ci.org/doctrine/dbal/jobs/273964540

You can run phpcbf and phpcs with specific files to fix/check them.

@greg0ire
Copy link
Member Author

I think that it could be possible to only check the new code, which @Ocramius would probably deem acceptable because it would allow slowly porting the code to the new style instead of creating a gazillion conflicts.

8000

@lcobucci
Copy link
Member

creating a gazillion conflicts

I like chaos HAHAHAHA

@greg0ire
Copy link
Member Author
greg0ire commented Sep 11, 2017

Could be achieved with something like https://github.com/greg0ire/git_template/blob/4877b9cc5786498eb89d2dfde6fc5c2e77e79c52/template/hooks/junkchecker/pre-commit#L18 , but I don't remember where I saw something like that with php-cs-fixer

@greg0ire greg0ire mentioned this pull request Sep 11, 2017
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
0