Skip to content

Emit event when running ./occ db:add-missing-indices - #9576

Merged
rullzer merged 1 commit into
masterfrom
feature/noid/missing_index_event
May 24, 2018
Merged

Emit event when running ./occ db:add-missing-indices#9576
rullzer merged 1 commit into
masterfrom
feature/noid/missing_index_event

Conversation

@rullzer

Copy link
Copy Markdown
Member

This allows apps to listen to this event in order to also update
indecies there.

Signed-off-by: Roeland Jago Douma roeland@famdouma.nl

* @param IDBConnection $connection
*/
public function __construct(IDBConnection $connection) {
public function __construct(IDBConnection $connection, EventDispatcherInterface $dispatcher) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You're missing the assignment of $dispatcher to the private field.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

look again

@rullzer
rullzerforce-pushed the feature/noid/missing_index_event branch from 0830a8f to 91ec554CompareMay 24, 2018 09:16
@codecov

codecovBot commented May 24, 2018

Copy link
Copy Markdown

Codecov Report

Merging #9576 into master will increase coverage by 22.04%.
The diff coverage is 0%.

@@ Coverage Diff @@## master #9576 +/- ##
=============================================
+ Coverage 29.13% 51.18% +22.04% 
Complexity 25673 25673 =============================================
Files 1568 1568 Lines 87950 87953 +3 =============================================
+ Hits 25623 45015 +19392 + Misses 62327 42938 -19389
Impacted FilesCoverage ΔComplexity Δ
core/register_command.php0% <0%> (ø)0 <0> (ø)⬇️
core/Command/Db/AddMissingIndices.php0% <0%> (ø)7 <1> (ø)⬇️
lib/private/Files/Stream/Encryption.php75.22% <0%> (+0.45%)54% <0%> (ø)⬇️
apps/theming/lib/ThemingDefaults.php89.79% <0%> (+0.68%)51% <0%> (ø)⬇️
lib/private/Setup.php12.55% <0%> (+0.83%)52% <0%> (ø)⬇️
lib/public/AppFramework/Db/Mapper.php94.95% <0%> (+0.84%)30% <0%> (ø)⬇️
lib/private/Group/Group.php88.88% <0%> (+0.92%)52% <0%> (ø)⬇️
...vate/Authentication/Token/DefaultTokenProvider.php98.03% <0%> (+0.98%)32% <0%> (ø)⬇️
lib/private/Preview/Generator.php78.03% <0%> (+1.15%)52% <0%> (ø)⬇️
lib/private/App/DependencyAnalyzer.php95.62% <0%> (+1.25%)64% <0%> (ø)⬇️
... and 378 more

@MorrisJobkeMorrisJobke left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Works 👍

@MorrisJobkeMorrisJobke added 4. to release Ready to be released and/or waiting for tests to finish and removed 3. to review Waiting for reviews labels May 24, 2018
@MorrisJobke

Copy link
Copy Markdown
Member

Retriggered CI job

Comment threadlib/public/IDBConnection.php Outdated
*/
interface IDBConnection {

const ADD_MISSING_INDEXES_EVENT = this::class . '::ADD_MISSING_INDEXES';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

lib/public/IDBConnection.php:49 PhanUndeclaredClassConstant Reference to constant class from undeclared class \OCP\this

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I fixed it by replacing the this with self 😉

This allows apps to listen to this event in order to also update
indecies there.
Signed-off-by: Roeland Jago Douma <roeland@famdouma.nl>
@MorrisJobke
MorrisJobkeforce-pushed the feature/noid/missing_index_event branch from 91ec554 to 80cc8d0CompareMay 24, 2018 16:03
@rullzer
rullzer merged commit eefe256 into masterMay 24, 2018
@rullzer
rullzer deleted the feature/noid/missing_index_event branch May 24, 2018 18:33
@rullzer

Copy link
Copy Markdown
MemberAuthor

Would a backport make sense? it is a small change but then apps can already add indexes for 13 apps.

@MorrisJobke

Copy link
Copy Markdown
Member

Would a backport make sense? it is a small change but then apps can already add indexes for 13 apps.

I would say so.

@rullzer

Copy link
Copy Markdown
MemberAuthor

backport in #9620

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

Labels

4. to releaseReady to be released and/or waiting for tests to finishenhancement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@rullzer@MorrisJobke@juliusknorr