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

[Cache][DoctrineBridge][Lock][Messenger] Add parameter $isSameDatabase to configureSchema() methods #50689

Conversation

alli83
Copy link
Contributor

@alli83 alli83 commented Jun 16, 2023

Q A
Branch? 7.0
Bug fix? no
New feature? no
Deprecations? no
Tickets
License MIT
Doc PR

$isSameDatabase parameter was introduced in #48059.
This PR aims to remove BC layer on configureSchema method and $isSameDatabase parameter

This parameter has been added in UPGRADE-6.3 in #50699

@carsonbot
Copy link

Hey!

Thanks for your PR. You are targeting branch "7.0" but it seems your PR description refers to branch "7".
Could you update the PR description or change target branch? This helps core maintainers a lot.

Cheers!

Carsonbot

@carsonbot carsonbot changed the title [DoctrineBridge]rm BC layer configureSchema check database [DoctrineBridge] rm BC layer configureSchema check database Jun 16, 2023
@alli83 alli83 force-pushed the doctrine-bridge-rm-bc-layer-configure-schema branch 2 times, most recently from cb8eb72 to 0f97b92 Compare June 16, 2023 15:13
@chalasr
Copy link
Member

chalasr commented Jun 16, 2023

The fact there is no UPGRADE entry added in this PR nor in the 6.3 PR looks suspicious to me as it's a breaking change that used to trigger deprecation notices on 6.3. Either there's a good reason not to mention it that I'm not aware of, or we just forgot to do it and then it should be fixed up

@alli83
Copy link
Contributor Author

alli83 commented Jun 18, 2023

@chalasr thank you for your feedback. I made an other PR that targets 6.3 and aims to add the missing notes in UPGRADE-6.3.

nicolas-grekas added a commit that referenced this pull request Jun 21, 2023
…d update tests (alli83)

This PR was merged into the 6.3 branch.

Discussion
----------

[DoctrineBridge] add missing UPGRADE notes for #50689 and update tests

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | yes
| Tickets       |
| License       | MIT
| Doc PR        |

`$isSameDatabase` parameter was introduced in #48059. It has been added in order to know whether the database used by doctrine is the same as the one used by the component when integrating the latter with doctrine migrations.

Also related to #50689

Commits
-------

5d2817d [DoctrineBridge] add missing UPGRADE notes for #50689
nicolas-grekas added a commit that referenced this pull request Jun 21, 2023
* 6.3:
  [Validator] Add missing validator translations in Polish language
  [HttpClient] Fix encoding some characters in query strings
  [HttpKernel] make RequestPayloadValueResolver:resolve() throw on variadic argument
  [SecurityBundle] Remove last usages of tag `security.remember_me_aware`
  [VarDumper] Dumping DateTime throws error if getTimezone is false
  Only update autoload_runtime.php when it changed
  [FrameworkBundle] Fix secrets:list not displaying local vars
  [Intl] Update the ICU data to 73.2
  [DoctrineBridge] add missing UPGRADE notes for #50689
  [HttpClient] Force int conversion for floated multiplier for GenericRetryStrategy
  [Security] Fix log message in OidcTokenHandler
  Don't mark RedispatchMessage as internal
  [FrameworkBundle] Ignore missing directories in about command
  Revert "[Messenger] Respect `isRetryable` decision of the retry strategy when deciding if failed message should be re-delivered"
  [Validator] Add the `message` option to the `PasswordStrength` constraint
  [Validator][Translator] Fix xlf files for en & fr translations. Bug introduced by #50590
  CS fix
  Add missing EN and FR translations for newest constraints
  [HttpClient] Remove final keyword on AsyncResponse
  [DependencyInjection] Fix support for `false` boolean env vars
nicolas-grekas added a commit that referenced this pull request Jun 21, 2023
* 6.4: (32 commits)
  [Validator] Add missing validator translations in Polish language
  [FrameworkBundle][Workflow] Add metadata dumping support for GraphvizDumper
  [HttpClient] Fix encoding some characters in query strings
  [HttpKernel] make RequestPayloadValueResolver:resolve() throw on variadic argument
  Fix typos
  Added redlink notifier
  [SecurityBundle] Remove last usages of tag `security.remember_me_aware`
  [VarDumper] Dumping DateTime throws error if getTimezone is false
  [DependencyInjection] Deprecate `ContainerAwareInterface`, `ContainerAwareTrait` and `ContainerAwareLoader`
  Only update autoload_runtime.php when it changed
  [Routing] Fix version in CHANGELOG
  [Console] Aligned multiline text in vertical table
  Fix README
  [Notifier] add Ntfy bridge
  [FrameworkBundle] Fix secrets:list not displaying local vars
  [Intl] Update the ICU data to 73.2
  [DoctrineBridge] add missing UPGRADE notes for #50689
  [HttpClient] Force int conversion for floated multiplier for GenericRetryStrategy
  [Security] Fix log message in OidcTokenHandler
  [Notifier] Add Novu bridge
  ...
@nicolas-grekas
Copy link
Member

Rebase unlocked. Please add lines to the UPGRADE+CHANGELOG files stating that a new argument has been added to the corresponding methods also.

@alli83 alli83 force-pushed the doctrine-bridge-rm-bc-layer-configure-schema branch from 0f97b92 to 098a747 Compare June 26, 2023 15:37
@alli83
Copy link
Contributor Author

alli83 commented Jun 26, 2023

Rebase unlocked. Please add lines to the UPGRADE+CHANGELOG files stating that a new argument has been added to the corresponding methods also.

I open a new PR on 6.3 for the missing CHANGELOG mention ?

nicolas-grekas added a commit that referenced this pull request Jun 26, 2023
…eDatabase arg… (alli83)

This PR was merged into the 6.3 branch.

Discussion
----------

[DoctrineBridge] add missing changelog mention for isSameDatabase arg…

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

$isSameDatabase parameter was introduced in #48059. It has been added in order to know whether the database used by doctrine is the same as the one used by the component when integrating the latter with doctrine migrations.

Also related to #50699 (UPGRADE mention) and #50689

Commits
-------

208861a [DoctrineBridge] add missing changelog mention for isSameDatabase arg introduced in #48059
@alli83 alli83 force-pushed the doctrine-bridge-rm-bc-layer-configure-schema branch 2 times, most recently from 14fa83e to ad9d6dd Compare June 27, 2023 16:10
alli83 added a commit to alli83/symfony that referenced this pull request Jun 27, 2023
@alli83 alli83 force-pushed the doctrine-bridge-rm-bc-layer-configure-schema branch from ad9d6dd to 7a8a8bf Compare June 27, 2023 16:46
@nicolas-grekas nicolas-grekas changed the title [DoctrineBridge] rm BC layer configureSchema check database [DoctrineBridge] Add parameter $isSameDatabase to configureSchema() methods Jun 27, 2023
@carsonbot carsonbot changed the title [DoctrineBridge] Add parameter $isSameDatabase to configureSchema() methods [Cache][DoctrineBridge][Lock][Messenger] Add parameter $isSameDatabase to configureSchema() methods Jun 27, 2023
@chalasr
Copy link
Member

chalasr commented Jun 28, 2023

I open a new PR on 6.3 for the missing CHANGELOG mention ?

Ideally yes, please. Wanna-be-added parameters trigger deprecation notices in case of inheritance and direct calls, so they should be documented as any other deprecation

@alli83
Copy link
Contributor Author

alli83 commented Jun 28, 2023

I open a new PR on 6.3 for the missing CHANGELOG mention ?

Ideally yes, please. Wanna-be-added parameters trigger deprecation notices in case of inheritance and direct calls, so they should be documented as any other deprecation

@chalasr , an other PR for missing changeling and upgrade 6.3 #50798

nicolas-grekas added a commit that referenced this pull request Jun 28, 2023
…0689 (alli83)

This PR was merged into the 6.3 branch.

Discussion
----------

[Lock] add missing UPGRADE and CHANGELOG Lock mention #50689

| Q             | A
| ------------- | ---
| Branch?       | 6.3
| Bug fix?      | no
| New feature?  | no
| Deprecations? | no
| Tickets       |
| License       | MIT
| Doc PR        |

$isSameDatabase parameter for DoctrineDbalStore was added in #48999.

Also related to #50689

Commits
-------

147067d [Lock] add missing UPGRADE and CHANGELOG Lock mention #50689
@nicolas-grekas
Copy link
Member

Thank you @alli83.

@nicolas-grekas nicolas-grekas merged commit 58f1ca5 into symfony:7.0 Jun 28, 2023
7 checks passed
nicolas-grekas added a commit that referenced this pull request Jul 5, 2023
* 6.3:
  [Scheduler] Fix `PeriodicalTrigger` from argument for stateful run dates
  cs fix
  [Messenger] Fix passing options set via tags to handler descriptors
  random_bytes length should be an int greater than 0
  enforce UTC timezone in test
  [DependencyInjection] Fix autocasting null env values to empty string
  Fix executable bit
  Fix executable bit
  Readme: Replace Stack Overflow with GitHub Discussions
  [DoctrineBridge] Remove outdated comment
  [DependencyInjection] Fix annotation
  [DoctrineBridge] Improve subscriber deprecation message
  [SecurityBundle] Do not translate `Bearer` header’s `error_description`
  [Lock] add missing UPGRADE and CHANGELOG Lock mention #50689
  [String] Fix Inflector for 'status'
  [DependencyInjection] Fix resource tracking for lazy services
  [EventDispatcher] [EventDispatcher] Throw exception when listener method cannot be resolved
  [Serializer] Fix type error not be accessed before initialization
nicolas-grekas added a commit that referenced this pull request Jul 5, 2023
* 6.4:
  [Scheduler] Fix `PeriodicalTrigger` from argument for stateful run dates
  cs fix
  [Messenger] Fix passing options set via tags to handler descriptors
  random_bytes length should be an int greater than 0
  enforce UTC timezone in test
  [DependencyInjection] Fix autocasting null env values to empty string
  Fix executable bit
  Fix executable bit
  Readme: Replace Stack Overflow with GitHub Discussions
  [DoctrineBridge] Remove outdated comment
  [DependencyInjection] Fix annotation
  [DoctrineBridge] Improve subscriber deprecation message
  [SecurityBundle] Do not translate `Bearer` header’s `error_description`
  [Lock] add missing UPGRADE and CHANGELOG Lock mention #50689
  [String] Fix Inflector for 'status'
  [DependencyInjection] Fix resource tracking for lazy services
  [EventDispatcher] [EventDispatcher] Throw exception when listener method cannot be resolved
  [Serializer] Fix type error not be accessed before initialization
@fabpot fabpot mentioned this pull request Oct 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants