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

Add missing generated option to documentation #11613

Merged
merged 1 commit into from
Dec 11, 2024
Merged

Add missing generated option to documentation #11613

merged 1 commit into from
Dec 11, 2024
+1 −0

Conversation

alexander-schranz
Copy link
Contributor

Seems not be documented in: #9118

alexander-schranz
alexander-schranz commented Sep 26, 2024
View reviewed changes
docs/en/reference/basic-mapping.rst
@@ -239,6 +239,7 @@ Here is a complete list of ``Column``s attributes (all optional):
- ``nullable`` (default: ``false``): Whether the column is nullable.
- ``insertable`` (default: ``true``): Whether the column should be inserted.
- ``updatable`` (default: ``true``): Whether the column should be updated.
- ``generated`` (default: ``null``): Whether the generated strategy should be ``'NEVER'``, ``'INSERT'`` and ``ALWAYS``.
Copy link
Contributor Author

@alexander-schranz alexander-schranz Sep 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

not sure if we should document NEVER as for me it doesn't make lot of sense if default is null doing nothing. But it was added to PHPDoc:

orm/src/Mapping/Column.php

Line 18 in cfc0655

* @psalm-param 'NEVER'|'INSERT'|'ALWAYS'|null $generated

/cc @mehldau

Copy link
Member

Choose a reason for hiding this comment

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

NEVER is a valid value so it should be mentioned. A default may change over time.

@alexander-schranz alexander-schranz changed the title Add missing generated option Add missing generated option to documentation Sep 26, 2024
SenseException
SenseException previously approved these changes Sep 26, 2024
View reviewed changes
docs/en/reference/basic-mapping.rst
@@ -239,6 +239,7 @@ Here is a complete list of ``Column``s attributes (all optional):
- ``nullable`` (default: ``false``): Whether the column is nullable.
- ``insertable`` (default: ``true``): Whether the column should be inserted.
- ``updatable`` (default: ``true``): Whether the column should be updated.
- ``generated`` (default: ``null``): Whether the generated strategy should be ``'NEVER'``, ``'INSERT'`` and ``ALWAYS``.
Copy link
Member

Choose a reason for hiding this comment

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

NEVER is a valid value so it should be mentioned. A default may change over time.

@greg0ire
Copy link
Member

Please kindly squash your commits together. If you don't, we'll try to remember to do it for you but it's best if you save us this trouble.

How to do that?

  1. git rebase -i origin/2.19.x, assuming origin is a git remote that points to this repository, and not your fork. If you're not sure what your remotes are, run git remote -vvv, there should be your fork and the holy/reference/base/origin/whatever-you-call-it repository.
  2. A window will show up with many lines, replace pick with fixup on every line but the first one
  3. Close your editor, git should do its magic, and you should end up with one commit
  4. Use git push --force to overwrite what you already push. Don't forget the --force option otherwise git will try to merge both things together.

@alexander-schranz
Copy link
Contributor Author

alexander-schranz commented Dec 11, 2024 •
edited
Loading

@greg0ire done. PS you can also enable squash or rebase merge in the Github UI in the Settings to merge a pull request via squash or rebased methods.

@greg0ire
Copy link
Member

I know but again…

we'll try to remember to do it for you but it's best if you save us this trouble.

I cannot guess how you are going to combine the commit messages together.

@greg0ire greg0ire changed the base branch from 2.19.x to 2.20.x December 11, 2024 18:50
@greg0ire greg0ire dismissed SenseException’s stale review December 11, 2024 18:50

The base branch was changed.

@greg0ire greg0ire added this to the 2.20.1 milestone Dec 11, 2024
@greg0ire greg0ire merged commit 53b51ae into doctrine:2.20.x Dec 11, 2024
1 check passed
@greg0ire
Copy link
Member

Thanks @alexander-schranz

@alexander-schranz alexander-schranz deleted the patch-2 branch December 11, 2024 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire approved these changes

@SenseException SenseException SenseException left review comments

Assignees
No one assigned
Labels
Documentation
Projects
None yet
Milestone
2.20.1
Development

Successfully merging this pull request may close these issues.
3 participants
@alexander-schranz @greg0ire @SenseException