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

fix default values and type hints for GroupManager::search #36592

Merged
merged 4 commits into from
May 12, 2023
Merged

fix default values and type hints for GroupManager::search #36592

merged 4 commits into from
May 12, 2023

Conversation

icewind1991
Copy link
Member

ints really are ints

Signed-off-by: Robin Appelman robin@icewind.nl

@icewind1991 icewind1991 added the 3. to review Waiting for reviews label Feb 7, 2023
@icewind1991 icewind1991 added this to the Nextcloud 26 milestone Feb 7, 2023
@icewind1991 icewind1991 requested review from a team, ArtificialOwl, blizzz and come-nc and removed request for a team February 7, 2023 16:22
@icewind1991 icewind1991 mentioned this pull request Feb 7, 2023
Merged
@icewind1991 icewind1991 force-pushed the groupmanager-search-typing branch from d1c8118 to 5871ca0 Compare February 7, 2023 16:42
@icewind1991 icewind1991 force-pushed the groupmanager-search-typing branch from 42cb044 to 1e1275b Compare February 8, 2023 10:46
@icewind1991 icewind1991 modified the milestones: Nextcloud 26, Nextcloud 27 Feb 9, 2023
@icewind1991
Copy link
Member Author

changed the IGroupManager to accept nulls to prevent messing with code that relied on passing null as a default value, the nulls are changed to the proper -/0 before passing them to the backends so the backends still get the stronger typing

@icewind1991 icewind1991 requested a review from come-nc February 14, 2023 12:31
come-nc
come-nc approved these changes Feb 14, 2023
View reviewed changes
Copy link
Contributor

@come-nc come-nc left a comment

Choose a reason for hiding this comment

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

See nitpick, fine otherwise.

lib/private/Group/Database.php Outdated Show resolved Hide resolved
This was referenced May 3, 2023
Merged
Closed
Merged
@skjnldsv skjnldsv added bug 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 9, 2023
@icewind1991 icewind1991 force-pushed the groupmanager-search-typing branch from 762999f to 36703b7 Compare May 11, 2023 12:31
icewind1991 and others added 4 commits May 11, 2023 17:30
@icewind1991
fix default values and type hints for GroupManager::search
e4b9ecd 
ints really are ints

Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
also update groupinterface and database backend
7ad3574 
Signed-off-by: Robin Appelman <robin@icewind.nl>
@icewind1991
revert public interface back to the original types, just cast them be…
13b06ba 
…fore passing the nulls to the backends

Signed-off-by: Robin Appelman <robin@icewind.nl>
@skjnldsv @come-nc @icewind1991
fix: getGroups limit check syntax
4a86487 
Co-authored-by: Côme Chilliet <91878298+come-nc@users.noreply.github.com>
Signed-off-by: John Molakvoæ <skjnldsv@users.noreply.github.com>
@icewind1991 icewind1991 force-pushed the groupmanager-search-typing branch from 36703b7 to 4a86487 Compare May 11, 2023 15:31
@icewind1991 icewind1991 merged commit 1f4dd62 into master May 12, 2023
@icewind1991 icewind1991 deleted the groupmanager-search-typing branch May 12, 2023 13:25
@ChristophWurst ChristophWurst added the pending documentation This pull request needs an associated documentation update label Jun 5, 2023
@skjnldsv skjnldsv mentioned this pull request Oct 17, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@come-nc come-nc come-nc approved these changes

@skjnldsv skjnldsv skjnldsv approved these changes

@ArtificialOwl ArtificialOwl Awaiting requested review from ArtificialOwl ArtificialOwl was automatically assigned from nextcloud/server-backend

@blizzz blizzz Awaiting requested review from blizzz blizzz was automatically assigned from nextcloud/server-backend

Assignees
No one assigned
Labels
4. to release Ready to be released and/or waiting for tests to finish bug pending documentation This pull request needs an associated documentation update
Projects
None yet
Milestone
Nextcloud 27
Development

Successfully merging this pull request may close these issues.
4 participants
@icewind1991 @skjnldsv @come-nc @ChristophWurst