Page MenuHomePhabricator

Redact key from error messages.
AbandonedPublic

Authored by Samwilson on Nov 1 2018, 1:44 AM.

Details

Reviewers
MusikAnimal
MaxSem
aezell
kaldari
Commits
R1966:529bbf4f1399: Redact key from error messages.
Patch without arc
git checkout -b D1124 && curl -L https://phabricator.wikimedia.org/D1124?download=true | git apply
Summary

Bug: T208374

Diff Detail

Repository
R1966 tool-ws-google-ocr
Branch
document-text-detection
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 3283
Build 5444: arc lint + arc unit

Event Timeline

Samwilson requested review of this revision.Nov 1 2018, 1:44 AM
Samwilson created this revision.
MaxSem added inline comments.Nov 1 2018, 2:04 AM
src/Ocr.php
77

I don't like that stack trace will be lost here.

Samwilson updated this revision to Diff 2931.Nov 1 2018, 3:48 AM
  • Include previous exception
Samwilson marked an inline comment as done.Nov 1 2018, 3:50 AM
Samwilson added inline comments.
src/Ocr.php
77

Yeah, me neither. I meant to at least put the previous exception. Done now.

I'm pretty sure there's a better way of doing what we want here, but I didn't want to spend too long on this. :)

Shall we re-write in Symfony?

Samwilson marked an inline comment as done.Nov 1 2018, 3:51 AM
kaldari accepted this revision.Nov 1 2018, 10:40 PM
This revision is now accepted and ready to land.Nov 1 2018, 10:40 PM
Samwilson updated this revision to Diff 3046.Wed, Mar 18, 2:02 AM
  • Switch to 'document text detection' instead of 'text detection'

Given that we are trying to minimize risky changes, this feels like something we should wait on.

While our testing says this is OK, our users expect the tool to behave in the way it currently does. Changing that behavior right now seems like a change we'd want to communicate more broadly before it happens.

Samwilson abandoned this revision.Wed, Mar 18, 2:30 AM

Good point!

And I've stuffed up arc diff in someway too... I'm not sure why this has got commits from earlier in it. We'll move to Gerrit before any more changes anyway...