Skip to content

Refactor comments app - #39236

Merged
skjnldsv merged 2 commits into
nextcloud:masterfrom
shdehnavi:refactor_comments_app
Sep 28, 2023
Merged

Refactor comments app#39236
skjnldsv merged 2 commits into
nextcloud:masterfrom
shdehnavi:refactor_comments_app

Conversation

@shdehnavi

@shdehnavishdehnavi commented Jul 7, 2023

Copy link
Copy Markdown
Contributor

Summary

The required adjustments have been made to the comments app.

The improvements:

  • Using PHP8's constructor property promotion
  • Adding return types
  • Adding types to properties
  • Updating doc blocks
  • Converting isset ternary to null coalescing operator

Checklist

@solracsfsolracsf added this to the Nextcloud 28 milestone Jul 9, 2023
@solracsfsolracsf added 3. to review Waiting for reviews technical debt 🧱 🤔🚀 labels Jul 9, 2023
@shdehnavishdehnavi mentioned this pull request Jul 13, 2023
Merged
nickvergessen
nickvergessen requested changes Jul 17, 2023
View reviewed changes
Comment threadapps/comments/lib/Activity/Listener.php Outdated
Comment threadapps/comments/lib/Activity/Provider.php Outdated
Comment threadapps/comments/lib/Controller/NotificationsController.php Outdated
Comment threadapps/comments/lib/Search/Result.php Outdated
shdehnaviand others added 2 commits September 28, 2023 09:16
@shdehnavi@fsamapoor
Refactor comments app
8782880
Signed-off-by: Hamid Dehnavi <hamid.dev.pro@gmail.com>
@shdehnavi@nickvergessen@fsamapoor
Make adjustments based on the review
92b75bc
Co-authored-by: Joas Schilling <coding@schilljs.com>
Signed-off-by: Hamid Dehnavi <hamid.dev.pro@gmail.com>
@skjnldsvskjnldsv added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels Sep 28, 2023
@skjnldsv
skjnldsv merged commit 456aea8 into nextcloud:masterSep 28, 2023
@skjnldsv

skjnldsv commented Sep 28, 2023

Copy link
Copy Markdown
Member

Cypress fails because fork master is not up to date, merging...

@fsamapoor

fsamapoor commented Sep 28, 2023•
edited
Loading

Copy link
Copy Markdown
Member

Cypress fails because fork master is not up to date, merging...

Thank you for taking the time to explain why the Cypress workflows have been failing for the past weeks.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nickvergessennickvergessennickvergessen approved these changes
@fsamapoorfsamapoorfsamapoor approved these changes
@skjnldsvskjnldsvskjnldsv approved these changes
@juliusknorrjuliusknorrAwaiting requested review from juliusknorr

Assignees

No one assigned

Labels

4. to releaseReady to be released and/or waiting for tests to finishtechnical debt🧱 🤔🚀

Projects

None yet

Milestone

Nextcloud 28

Development

Successfully merging this pull request may close these issues.

5 participants

@shdehnavi@skjnldsv@fsamapoor@nickvergessen@solracsf