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

[WIP] Adding new symfony events to replace old hooks #30268

Closed
wants to merge 1 commit into from

Conversation

sharidas
Copy link
Contributor

@sharidas sharidas commented Jan 25, 2018

Adding new symfony events for the following:

  1. setPassword
  2. setPassphrase

Signed-off-by: Sujith H [email protected]

Description

There are more places where old hooks are being used. The intention of this PR is to hunt them down and replace with the symfony events.

Related Issue

Motivation and Context

Replace the old private hooks with the symfony events.

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@sharidas sharidas added this to the development milestone Jan 25, 2018
@sharidas sharidas self-assigned this Jan 25, 2018
@sharidas
Copy link
Contributor Author

sharidas commented Jan 25, 2018

There is a followup for this PR in the app: owncloud/encryption#25
The activity app also modified: owncloud/activity#608

@codecov
Copy link

codecov bot commented Jan 25, 2018

Codecov Report

Merging #30268 into master will decrease coverage by 45.72%.
The diff coverage is n/a.

Impacted file tree graph

@@              Coverage Diff              @@
##             master   #30268       +/-   ##
=============================================
- Coverage     61.54%   15.82%   -45.73%     
+ Complexity    18500      920    -17580     
=============================================
  Files          1090       51     -1039     
  Lines         61084     3356    -57728     
=============================================
- Hits          37593      531    -37062     
+ Misses        23491     2825    -20666
Impacted Files Coverage Δ Complexity Δ
lib/private/Files/Storage/DAV.php 6.06% <0%> (-82.76%) 163% <0%> (ø)
apps/files_external/lib/AppInfo/Application.php 0% <0%> (ø) 4% <0%> (ø) ⬇️
apps/files_external/templates/list.php 0% <0%> (ø) 0% <0%> (ø) ⬇️
lib/private/legacy/template.php
lib/private/Preview/Image.php
core/Command/User/ListUsers.php
lib/private/Files/Stream/Encryption.php
lib/private/Migration/BackgroundRepair.php
lib/private/Session/Internal.php
lib/private/Files/Config/CachedMountInfo.php
... and 1024 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8344a10...75893f8. Read the comment docs.

@sharidas sharidas force-pushed the new-symfony-events branch 13 times, most recently from e958729 to 2f4cf9c Compare February 12, 2018 06:33
@sharidas sharidas force-pushed the new-symfony-events branch 2 times, most recently from 1e60b73 to db5df19 Compare February 12, 2018 12:51
Adding new symfony events for the following:
1) setPassword
2) setPassphrase

Signed-off-by: Sujith H <[email protected]>
@PVince81
Copy link
Contributor

please don't delete the old hooks, we want to stay compatible

the event dispatcher events are an addition

@felixboehm felixboehm modified the milestones: development, planned Apr 10, 2018
@PVince81
Copy link
Contributor

obsolete ?

@PVince81
Copy link
Contributor

tech debt to be rescheduled

@DeepDiver1975
Copy link
Member

As discussed in #35777 the master branch will from now on hold the ownCloud 10 codebase.

This PR targetted ownCloud 11 which is postponed to a far distant future.

Because of that I'm closing this PR and kindly ask you to re-submit this PR in a few days.

Thanks a lot for your patience

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants