Skip to content

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Nov 6, 2018

Fix #12058

@nextcloud/designers

@skjnldsv skjnldsv added bug design Design, UI, UX, etc. 3. to review Waiting for reviews feature: caching Related to our caching system: scssCacher, jsCombiner... labels Nov 6, 2018
@skjnldsv skjnldsv added this to the Nextcloud 15 milestone Nov 6, 2018
@skjnldsv skjnldsv self-assigned this Nov 6, 2018
@skjnldsv skjnldsv force-pushed the generated-avatar-major-cleanup branch from 959f092 to 78dd2ce Compare November 6, 2018 14:55
@MorrisJobke MorrisJobke mentioned this pull request Nov 6, 2018
29 tasks
Copy link
Member

@MorrisJobke MorrisJobke left a comment

Choose a reason for hiding this comment

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

Code makes sense 👍

rullzer
rullzer previously requested changes Nov 6, 2018
@skjnldsv skjnldsv force-pushed the generated-avatar-major-cleanup branch from 78dd2ce to a884be4 Compare November 7, 2018 09:39
@skjnldsv skjnldsv requested a review from rullzer November 7, 2018 09:43
@skjnldsv
Copy link
Member Author

skjnldsv commented Nov 7, 2018

Failure unrelated

rullzer
rullzer previously requested changes Nov 7, 2018
* @see IAvatar
* @since 15.0.0
*/
public function clearCachedAvatars();
Copy link
Member

Choose a reason for hiding this comment

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

What is this doing in the public namespace.
IMO we should just inject the private class in the repair step and keep this out of here. I see no reason for an app to trigger this.

@skjnldsv skjnldsv force-pushed the generated-avatar-major-cleanup branch from 87e581f to 1c13c34 Compare November 7, 2018 19:37
@skjnldsv skjnldsv dismissed rullzer’s stale review November 7, 2018 19:38

Please update

@skjnldsv skjnldsv requested a review from rullzer November 7, 2018 19:38
Signed-off-by: John Molakvoæ (skjnldsv) <[email protected]>
@skjnldsv skjnldsv force-pushed the generated-avatar-major-cleanup branch from 1c13c34 to a9eef37 Compare November 8, 2018 07:31
@rullzer rullzer merged commit 25fe324 into master Nov 8, 2018
@MorrisJobke MorrisJobke deleted the generated-avatar-major-cleanup branch November 8, 2018 09:41
new AddCleanupUpdaterBackupsJob(\OC::$server->getJobList()),
new RepairPendingCronJobs(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig()),
new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig()),
new SetVcardDatabaseUID(\OC::$server->getDatabaseConnection(), \OC::$server->getConfig())
Copy link
Member

Choose a reason for hiding this comment

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

We always kept a trailing , in arrays to make the diff for adding a new entry nicer. It then only has this one line added instead of also appending a , to the previous line.

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 bug design Design, UI, UX, etc. feature: caching Related to our caching system: scssCacher, jsCombiner...

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants