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

declare addressDataProperties to avoid PHP 8.2 deprecation notice #1446

Merged

Conversation

phil-davis
Copy link
Contributor

@phil-davis phil-davis commented Jan 30, 2023

Issue #1445

declare addressDataProperties to avoid PHP 8.2 deprecation notice

This is the easy fix that I can see for one of the things mentioned in the issue.

@codecov
Copy link

codecov bot commented Jan 30, 2023

Codecov Report

Merging #1446 (0ccc094) into master (406f688) will not change coverage.
Report is 2 commits behind head on master.
The diff coverage is n/a.

@@            Coverage Diff            @@
##             master    #1446   +/-   ##
=========================================
  Coverage     97.34%   97.34%           
  Complexity     2830     2830           
=========================================
  Files           175      175           
  Lines          9418     9418           
=========================================
  Hits           9168     9168           
  Misses          250      250           
Files Coverage Δ
.../CardDAV/Xml/Request/AddressBookMultiGetReport.php 100.00% <ø> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@phil-davis phil-davis self-assigned this Jan 30, 2023
@phil-davis phil-davis marked this pull request as ready for review January 30, 2023 12:17
Copy link
Member

@staabm staabm left a comment

Choose a reason for hiding this comment

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

Lgtm - can this be covered by a test?

@antiftw
Copy link

antiftw commented Jun 11, 2023

Any chance this might be merged in the near future? I just spent a day wondering and trying to figure out why DavX5 would not sync contacts anymore with my Baikal server after a reinstall.

Have now manually "fixed" the error by editing the code in the vendor map, but it would be nice for this to be merged to prevent issues in the future. Especially since it seems quite a low impact merge. Or I might be mistaken?

Thanks.

@jschwender
Copy link

I can confirm, that in my case this requested merge fixed the issue of broken synchronizing. I am running baikal 0.9.3 on Debian 12.0, with php 8.2 and DAVX5 4.3.4.1-ose on android 11. Now it works again (for me). Maybe that helps.

@Yannik
Copy link
Contributor

Yannik commented Jun 24, 2023

@phil-davis Would you mind merging this? :-)

@tbba
Copy link

tbba commented Aug 17, 2023

Please update.

@ByteHamster
Copy link
Member

I just solved the merge conflict. @phil-davis, what is the status of this PR? I'm getting quite a few bug reports about this over at Baikal. Can we get it merged? It does not look like it could break anything

@DeepDiver1975 DeepDiver1975 merged commit a74464d into sabre-io:master Oct 17, 2023
11 checks passed
@ByteHamster
Copy link
Member

Thanks @DeepDiver1975! Should I prepare a release commit like 2918524 for sabre/dav, so we can get this change into Baikal?

@DeepDiver1975
Copy link
Member

Let's discuss with @phil-davis and @staabm if there is more to be added. TBH I lost the track a bit in here .....

@phil-davis phil-davis deleted the deprecated-dynamic-properties branch November 14, 2023 08:50
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

8 participants