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

[Stable9.1] Backporting logging changes of PR #27144 #27239

Merged
merged 2 commits into from
Feb 27, 2017
Merged

[Stable9.1] Backporting logging changes of PR #27144 #27239

merged 2 commits into from
Feb 27, 2017

Conversation

noveens
Copy link
Contributor

@noveens noveens commented Feb 23, 2017

backporting logging changes of PR #27144

@mention-bot
Copy link

@noveens, thanks for your PR! By analyzing the history of the files in this pull request, we identified @icewind1991, @butonic and @DeepDiver1975 to be potential reviewers.

@noveens
Copy link
Contributor Author

noveens commented Feb 23, 2017

@PVince81 ,
Is this the right way to do backporting?

jvillafanez
jvillafanez reviewed Feb 24, 2017
View reviewed changes
lib/private/BackgroundJob/Job.php Outdated
$jobStartTime = time();

\OCP\Util::writeLog('cron', 'Started background job of class : ' . get_class($this) . ' with arguments : ' . print_r($this->argument, true), \OCP\Util::DEBUG
);
Copy link
Member

Choose a reason for hiding this comment

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

Move the ); up or split the line better.

lib/private/BackgroundJob/Job.php Outdated
$timeTaken = $jobEndTime - $jobStartTime;

\OCP\Util::writeLog('cron', "Finished background job, the job took : $timeTaken seconds, " . "this job is an instance of class : " . get_class($this) . ' with arguments : ' . print_r($this->argument, true), \OCP\Util::DEBUG
);
Copy link
Member

Choose a reason for hiding this comment

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

Move the ); up or split the line better.

@SergioBertolinSG SergioBertolinSG changed the title backporting logging changes of PR #27144 [Stable9.1] Backporting logging changes of PR #27144 Feb 24, 2017
@PVince81
Copy link
Contributor

Is this the right way to do backporting?

yes

@PVince81 PVince81 added this to the 9.1.5 milestone Feb 24, 2017
@noveens
Copy link
Contributor Author

noveens commented Feb 24, 2017

@jvillafanez @PVince81 ,
updated the changes.

@jvillafanez
Copy link
Member

👍

@noveens
Copy link
Contributor Author

noveens commented Feb 27, 2017

@PVince81 ,
merge this?

@PVince81 PVince81 merged commit d61600c into owncloud:stable9.1 Feb 27, 2017
@PVince81 PVince81 mentioned this pull request Feb 27, 2017
Merged
9 tasks
@jvillafanez
Copy link
Member

Looks like we overlooked a missing whitespace...

{"reqId":"69jAKwn4pVcPHAaw32ds","remoteAddr":"82.159.139.58","app":"cron","message":"Finished background job, the job took : 0 seconds, this job is an instance of class : OC\\Command\\CommandJobwith arguments : O:33:\"OCA\\Files_Trashbin\\Command\\Expire\":1:{s:39:\"\u0000OCA\\Files_Trashbin\\Command\\Expire\u0000user\";s:5:\"admin\";}","level":0,"time":"2017-02-27T12:24:35+00:00","method":"GET","url":"\/cron.php","user":"--"}

Note the Command\\CommandJobwith arguments

@noveens could you create another PR to fix this?

@noveens
Copy link
Contributor Author

noveens commented Feb 27, 2017

@jvillafanez ,
PR on the way.

@lock
Copy link

lock bot commented Aug 3, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 3, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jvillafanez jvillafanez jvillafanez left review comments

Assignees
No one assigned
Labels
3 - To Review
Projects
None yet
Milestone
9.1.5
Development

Successfully merging this pull request may close these issues.
4 participants
@noveens @mention-bot @PVince81 @jvillafanez