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

Fix for Deprecated function: Creation of dynamic property #415

Merged
merged 4 commits into from
Apr 18, 2024

Conversation

AWearring
Copy link
Contributor

Fixes issue #414

@stephen-cox
Copy link
Member

Github tests aren't running as we're not testing against the 4.x branch yet. This is fixed with #418.

If tests are fixed once #418 is merged this is good to be merged

@finnlewis
Copy link
Member

Note this is from https://www.drupal.org/project/autosave_form/issues/3355495

It would be good to include the issue number in the patch comment.

@finnlewis finnlewis self-assigned this Apr 9, 2024
@finnlewis
Copy link
Member

I'll test this today.

@finnlewis
Copy link
Member

Aha! @ekes has just pointed out that the issue has a more up to date solution on the merge request:

https://git.drupalcode.org/project/autosave_form/-/merge_requests/12/diffs#note_290984

So we don't really want to add the patch https://www.drupal.org/files/issues/2023-04-20/autosave_form-dynamic-property-deprecation.patch

So the changes we want are: https://git.drupalcode.org/project/autosave_form/-/merge_requests/12.patch

BUT it is not safe to create a patch from a merge request diff, as the merge request might change.

So we could create a patch file on the issue from the changes in that merge request.

Or we could encourage a new release (or both).

@finnlewis
Copy link
Member

@AWearring I've updated the patch to one that is the same as the merge request on that issue.

Are you able to test if this works for you?

@AWearring
Copy link
Contributor Author

@finnlewis I've built a fresh test controller site locally using this patch and I don't get the depreciation error, so it appears all good

Updating test runner.
@ekes
Copy link
Member

ekes commented Apr 16, 2024

Can I propose we merge this one. Get the one on microsite groups merged and then continue to tidy anything else that's failing in another issue/MR.

@finnlewis finnlewis merged commit 20e96d4 into 4.x Apr 18, 2024
5 of 8 checks passed
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.

None yet

4 participants