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.

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

Proofread types doc #2840

Merged
merged 4 commits into from
Oct 19, 2017
Merged

Proofread types doc #2840

merged 4 commits into from
Oct 19, 2017

Conversation

greg0ire
Copy link
Member

@greg0ire greg0ire commented Sep 3, 2017

This should help making #2839 look less messy, and is far more mergeable.

@@ -927,7 +927,10 @@ Now we implement our ``Doctrine\DBAL\Types\Type`` instance:
}
}

The job of Doctrine-DBAL is to transform your type into SQL declaration. You can modify the SQL declaration Doctrine will produce. At first, you must to enable this feature by overriding the canRequireSQLConversion method:
The job of Doctrine-DBAL is to transform your type into SQL declaration.
You can modify the SQL declaration Doctrine will produce. First, you
Copy link
Member

Choose a reason for hiding this comment

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

that Doctrine

@@ -927,7 +927,10 @@ Now we implement our ``Doctrine\DBAL\Types\Type`` instance:
}
}

The job of Doctrine-DBAL is to transform your type into SQL declaration. You can modify the SQL declaration Doctrine will produce. At first, you must to enable this feature by overriding the canRequireSQLConversion method:
The job of Doctrine-DBAL is to transform your type into SQL declaration.
Copy link
Member

Choose a reason for hiding this comment

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

And to convert values?

Copy link
Member

Choose a reason for hiding this comment

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

Or is that covered in the rest of this article?

Copy link
Member Author

Choose a reason for hiding this comment

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

It is covered just below, on line 943

@@ -937,7 +940,8 @@ The job of Doctrine-DBAL is to transform your type into SQL declaration. You can
return true;
}

Then you override the methods convertToPhpValueSQL and convertToDatabaseValueSQL :
Then you override the methods ``convertToPhpValueSQL`` and
Copy link
Member

Choose a reason for hiding this comment

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

methods to be put at the end of this sentence

@greg0ire
Copy link
Member Author

@Ocramius fixed, please review again

@greg0ire
Copy link
Member Author

continuousphp seems to be stuck... should I close and reopen?

@lcobucci
Copy link
Member

lcobucci commented Sep 11, 2017

@greg0ire just amending/rebasing should restart continuousphp

@greg0ire greg0ire closed this Sep 11, 2017
@greg0ire greg0ire reopened this Sep 11, 2017
@greg0ire
Copy link
Member Author

Clicking 2 buttons sounds easier :P

@greg0ire
Copy link
Member Author

It's unstuck :)

@greg0ire
Copy link
Member Author

ping @Ocramius

Copy link
Contributor

@Majkl578 Majkl578 left a comment

Choose a reason for hiding this comment

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

👍, maybe a single squashed commit instead of 6 would be cleaner though... 😇

@Ocramius
Copy link
Member

Ocramius commented Sep 20, 2017 via email

@greg0ire
Copy link
Member Author

@Majkl578 if the PR is easier to read like that then no squash is needed :P

@SenseException
Copy link
Member

I've got nothing to add for this PR. 👍

@greg0ire
Copy link
Member Author

Rebased.

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, thanks @greg0ire!

@Ocramius Ocramius added this to the 2.7 milestone Oct 19, 2017
@Ocramius Ocramius merged commit 09e072d into doctrine:master Oct 19, 2017
@greg0ire greg0ire deleted the proofread_types_doc branch October 19, 2017 12:58
@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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants