Noissue - Use Source Sans Pro font files from website-defaults

Noissue - Use Source Sans Pro font files from website-defaults

[ire, 2018-04-04 09:08:49]

Ready for review

[juliandoucette, 2018-04-04 14:31:43]

The latest patch doesn't apply for some reason :S

(I can't figure out why)

[juliandoucette, 2018-04-04 14:33:34]

On 2018/04/04 14:31:43, juliandoucette wrote:
> The latest patch doesn't apply for some reason :S
> 
> (I can't figure out why)

FWIW The code looks good to me - provided the fallback to website-default font
files works in production.

[ire, 2018-04-05 07:29:20]

On 2018/04/04 14:33:34, juliandoucette wrote:
> On 2018/04/04 14:31:43, juliandoucette wrote:
> > The latest patch doesn't apply for some reason :S
> > 
> > (I can't figure out why)
> 
> FWIW The code looks good to me - provided the fallback to website-default font
> files works in production.

I can't figure out what the issue is either. I just rebased so maybe try again?

[juliandoucette, 2018-04-05 10:49:11]

On 2018/04/05 07:29:20, ire wrote:
> I can't figure out what the issue is either. I just rebased so maybe try
again?

I get the following error when I try to apply this patch to a fresh clone of
https://hg.adblockplus.org/help.eyeo.com

error: patch failed: static/src/fonts/Source-Sans-Pro-600/LICENSE.txt:1
error: static/src/fonts/Source-Sans-Pro-600/LICENSE.txt: patch does not apply

Will you confirm?

[ire, 2018-04-05 11:44:06]

On 2018/04/05 10:49:11, juliandoucette wrote:
> On 2018/04/05 07:29:20, ire wrote:
> > I can't figure out what the issue is either. I just rebased so maybe try
> again?
> 
> I get the following error when I try to apply this patch to a fresh clone of
> https://hg.adblockplus.org/help.eyeo.com
> 
> error: patch failed: static/src/fonts/Source-Sans-Pro-600/LICENSE.txt:1
> error: static/src/fonts/Source-Sans-Pro-600/LICENSE.txt: patch does not apply
> 
> Will you confirm?

I'm getting the same error :/

I've tried a bunch of things, but I'll keep at it to see what the issue might
be.

[juliandoucette, 2018-04-05 11:52:59]

On 2018/04/05 11:44:06, ire wrote:
> I'm getting the same error :/
> 
> I've tried a bunch of things, but I'll keep at it to see what the issue might
> be.

Have you tried exporting the patch yourself and applying it to a fresh clone
instead of relying on reitveld/hgreview?

[ire, 2018-04-06 08:59:45]

On 2018/04/05 11:52:59, juliandoucette wrote:
> On 2018/04/05 11:44:06, ire wrote:
> > I'm getting the same error :/
> > 
> > I've tried a bunch of things, but I'll keep at it to see what the issue
might
> > be.
> 
> Have you tried exporting the patch yourself and applying it to a fresh clone
> instead of relying on reitveld/hgreview?

Yes, that works fine

[juliandoucette, 2018-04-06 11:55:33]

On 2018/04/06 08:59:45, ire wrote:
> Yes, that works fine

Will you send me the patch via email then so that I can test?

[ire, 2018-04-06 13:09:10]

On 2018/04/06 11:55:33, juliandoucette wrote:
> On 2018/04/06 08:59:45, ire wrote:
> > Yes, that works fine
> 
> Will you send me the patch via email then so that I can test?

Done

[juliandoucette, 2018-04-06 14:30:44]

LGTM