Skip to content

Conversation

@CarlSchwan
Copy link
Member

@CarlSchwan CarlSchwan commented Jan 15, 2026

Summary

This might also improve a bit the performance as we are spending 2-3% of CPU time in IShare::setId

TODO

  • ...

Checklist

@CarlSchwan CarlSchwan added this to the Nextcloud 33 milestone Jan 15, 2026
@CarlSchwan CarlSchwan self-assigned this Jan 15, 2026
@CarlSchwan CarlSchwan marked this pull request as ready for review January 15, 2026 17:41
@CarlSchwan CarlSchwan requested a review from a team as a code owner January 15, 2026 17:41
@CarlSchwan CarlSchwan requested review from ArtificialOwl, come-nc, leftybournes and provokateurin and removed request for a team January 15, 2026 17:41
@CarlSchwan CarlSchwan changed the title refactor(IShare): Add typing for node refactor(IShare): Add typing for IShare Jan 15, 2026
@nextcloud-bot nextcloud-bot mentioned this pull request Jan 15, 2026
@CarlSchwan CarlSchwan force-pushed the carl/typing-share branch 4 times, most recently from 8b6715c to cec0a3b Compare January 16, 2026 13:22
Copy link
Contributor

@artonge artonge left a comment

Choose a reason for hiding this comment

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

For v34?

@CarlSchwan CarlSchwan modified the milestones: Nextcloud 33, Nextcloud 34 Jan 16, 2026
@CarlSchwan
Copy link
Member Author

For v34?

Yeah feels tiny to risky to do that in 33. Not worth the tiny performance improvements

@CarlSchwan CarlSchwan marked this pull request as draft January 20, 2026 09:34
@CarlSchwan
Copy link
Member Author

Converting to draft until we do a branching as this is NC 34 only material

This might also improve a bit the performance.

Signed-off-by: Carl Schwan <carlschwan@kde.org>
@CarlSchwan CarlSchwan marked this pull request as ready for review January 27, 2026 10:19
@provokateurin provokateurin merged commit a17f4d4 into master Jan 27, 2026
199 of 203 checks passed
@provokateurin provokateurin deleted the carl/typing-share branch January 27, 2026 12:36
* @since 9.1.0
*/
public function setId($id);
public function setId(string $id): self;
Copy link
Member

Choose a reason for hiding this comment

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

Despite the code comment this was int everywhere as you can see in your code and test adjustments…
It breaks Talk share provider according to CI in richdocuments…

So this should receive a proper documentation PR and if possible we allow int for the time being and cast to string?

Copy link
Member

@juliusknorr juliusknorr Jan 28, 2026

Choose a reason for hiding this comment

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

Also breaking in deck it seems

@juliusknorr juliusknorr added the pending documentation This pull request needs an associated documentation update label Jan 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to review Waiting for reviews pending documentation This pull request needs an associated documentation update performance 🚀 technical debt

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants