Paths

Table of Contentst

Differential D8887

Use initializeNewLog rather than instantiate the UserLog
ClosedPublic
Actions

Authored by garoevans on Apr 28 2014, 10:17 PM.

Tags

None

Referenced Files

	Unknown Object (File)
	Wed, Jan 29, 4:42 AM

	Unknown Object (File)
	Mon, Jan 27, 7:28 AM

	Unknown Object (File)
	Mon, Jan 27, 6:15 AM

	Unknown Object (File)
	Mon, Jan 27, 6:15 AM

	Unknown Object (File)
	Mon, Jan 27, 5:37 AM

	Unknown Object (File)
	Sat, Jan 25, 7:48 AM

	Unknown Object (File)
	Tue, Jan 21, 9:13 AM

	Unknown Object (File)
	Sat, Jan 18, 3:50 AM

Subscribers

Tokens

"Doubloon" token, awarded by btrahan.

Details

Reviewers

Group Reviewers

Blessed Reviewers

Maniphest Tasks

T4912: Get certificate call returns exception following update

Commits

Restricted Diffusion Commit
rP08d9e5ec99ae: Use initializeNewLog rather than instantiate the UserLog

Summary

Use initializeNewLog rather than instantiate the UserLog,
Closes T4912

Test Plan

Run install-certificate

Diff Detail

Lint

Lint Skipped

Unit

Tests Skipped

Build Status

Buildable 104
Build 104: [Placeholder Plan] Wait for 30 Seconds

Event Timeline

garoevans updated this revision to Diff 21083.Apr 28 2014, 10:17 PM

garoevans retitled this revision from to Use initializeNewLog rather than instantiate the UserLog.

garoevans updated this object.

garoevans edited the test plan for this revision. (Show Details)

garoevans added a reviewer: btrahan.

garoevans set the repository for this revision to rP Phabricator.

garoevans added a task: T4912: Get certificate call returns exception following update.

garoevans added a subscriber: epriestley.

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptApr 28 2014, 10:17 PM

Harbormaster completed remote builds in B104: Diff 21083.Apr 28 2014, 10:18 PM

LMK if making a diff is a pain and I can make one you can accept.

src/applications/conduit/method/ConduitAPI_conduit_getcertificate_Method.php
62–66	This is technically different from what's on the left - $request->getUser() and the user from $info->getUserPHID() can be different people - but I think its more correct as $request->getUser() is actually the actor.
82	can you pass in the ConduitAPI $request here so you can do the $request->getUser() bit again? can you also pass in the PhabricatorConduitCertificateToken $info = null?
86	change to $info ? $info->getUserPHID() : '-'

This revision now requires changes to proceed.Apr 28 2014, 10:25 PM

btrahan awarded a token.Apr 28 2014, 10:25 PM

Will make other changes.

Made diff with git diff, not too bad :)

src/applications/conduit/method/ConduitAPI_conduit_getcertificate_Method.php
62–66	I thought the same Should have mentioned it in the commit. Probably the same person!

Attempt to log more info on failure

Nice! I can't see the top bit though in this diff. :/

Woops

Harbormaster completed remote builds in B106: Diff 21085.Apr 28 2014, 10:34 PM

btrahan added inline comments.Apr 28 2014, 10:34 PM

src/applications/conduit/method/ConduitAPI_conduit_getcertificate_Method.php
59	sorry, forgot to mention you have to update this to pass in $request and $info

Harbormaster completed remote builds in B107: Diff 21086.Apr 28 2014, 10:35 PM

Tested it properly

src/applications/conduit/method/ConduitAPI_conduit_getcertificate_Method.php
59	Ha, sorry. Being slow!

Harbormaster completed remote builds in B108: Diff 21087.Apr 28 2014, 10:39 PM

awesometown

This revision is now accepted and ready to land.Apr 28 2014, 10:42 PM

Closed by commit rP08d9e5ec99ae (authored by @btrahan).

Thanks for landing :)

no problemo. I think I messed up authorship info / I have a task to make that work better. I appreciate the commit! :D

Thanks!

Revision Contents
Changeset List

Path

Size

src/

applications/

conduit/

method/

ConduitAPI_conduit_getcertificate_Method.php

15 lines

Diff 21083

src/applications/conduit/method/ConduitAPI_conduit_getcertificate_Method.php

Loading...