Paths

Table of Contentst

Differential D17323

Add more phone numbers to "Shields Up" action
ClosedPublic
Actions

Authored by epriestley on Feb 7 2017, 8:27 PM.

Tags

Referenced Files

	Unknown Object (File)
	Tue, Dec 17, 5:34 AM

	Unknown Object (File)
	Tue, Dec 17, 12:39 AM

	Unknown Object (File)
	Sat, Dec 14, 3:26 PM

	Unknown Object (File)
	Sun, Dec 8, 10:21 AM

	Unknown Object (File)
	Sat, Dec 7, 6:25 PM

	Unknown Object (File)
	Thu, Dec 5, 10:14 AM

	Unknown Object (File)
	Fri, Nov 29, 10:11 PM

	Unknown Object (File)
	Wed, Nov 27, 4:59 PM

Subscribers

Details

Reviewers

Commits

R25:515f367b0e36: Add more phone numbers to "Shields Up" action

Summary

This diff adds phone numbers that were giving us trouble on https://developer.blender.org/

The specific lists of task that included these numbers can be found at https://developer.blender.org/search/query/RVlO.cUv.asA/#R

Test Plan

Create task with these phone numbers

Also, added test coverage.

Diff Detail

Repository

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Blendify created this revision.Feb 7 2017, 8:27 PM

Herald added a reviewer: Blessed Reviewers. · View Herald TranscriptFeb 7 2017, 8:27 PM

Herald added a subscriber: Korvin. · View Herald Transcript

Let's really AMP THIS UP to the NEXT LEVEL 🚀

src/abuse/SecureSheldsUpAction.php
112	Are these "O" ("oh"), not "0" ("zero")? I think the logic below will never match "O" ("oh"). Maybe do this: Put "0" ("zero") in the patterns. Make the logic below replace all "O" ("oh") with "0" ("zero") before performing the match (if ambitious: also replace "i", "I", "l", "L", "\|" with "1"). While you're here, fix the spelling of "betwee" a few lines below this.

This revision now requires changes to proceed.Feb 7 2017, 8:56 PM

I changed some things you requested but I do not have enough knowledge of php to do the other part. Maybe use str_replace? Anyway... feel free to commandeer this and make the change

Out of curiosity, why are there "Context not available." indicators here? Am I not supposed to see R25 Secure?

Screen Shot 2017-02-07 at 5.54.21 PM.png (1×2 px, 337 KB)

My guess is it was a cut & paste diff

Aha that makes sense. Thanks.

Yes, I just copied a pasted the diff. I was having an issue getting arc working for https://secure.phabricator.com/

I'll make the other adjustments -- str_replace() is the right approach, but this vital production infrastructure could probably use some unit tests anyway.

Add unit tests.
Match "o" and "O" for 0, etc.

Harbormaster completed remote builds in B15610: Diff 41711.Feb 13 2017, 3:55 PM

chad accepted this revision.Feb 13 2017, 4:12 PM

This revision is now accepted and ready to land.Feb 13 2017, 4:12 PM

Closed by commit R25:515f367b0e36: Add more phone numbers to "Shields Up" action (authored by epriestley, committed by epriestley). · Explain WhyFeb 13 2017, 4:46 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

8 lines

src/

__phutil_library_map__.php

2 lines

abuse/

SecureSheldsUpAction.php

31 lines

__tests__/

SecureSheldsUpTestCase.php

38 lines

Diff 41712

.arcunit

Loading...

src/__phutil_library_map__.php

Loading...

src/abuse/SecureSheldsUpAction.php

Loading...

src/abuse/tests/SecureSheldsUpTestCase.php

Loading...