Carbonalyser icon indicating copy to clipboard operation
Carbonalyser copied to clipboard

Published 20 hours ago • verified publisher icon carbonalyser

Ignore cache requests & no track incognito host

Open Jolg42 opened this issue 5 years ago • 8 comments

Fixes #12 as requests fromCache: true are noew ignored.

Replace host name by Incognito if the request was made in Incognito mode.

Voilà 😄

Jolg42 avatar Sep 06 '19 21:09 Jolg42

Hi @Jolg42 and thanks for this contribution. I will test it as soon as possible.

supertanuki avatar Sep 12 '19 13:09 supertanuki

This is in deed very important in order to have meaningful results. Thanks @Jolg42 I can't wait to see the difference. Meanwhile, however it's great work @supertanuki I believe it should be mentioned somewhere that these data are not be taken "as is" for the moment.

ImaCrea avatar Oct 01 '19 13:10 ImaCrea

Is there any problem holding the PR back? It should make quite a difference on the estimated footprint.

pveber avatar Jan 02 '20 11:01 pveber

We will take a look on this PR. There is a conflict to revolve due to the chrome support code modification

supertanuki avatar Feb 05 '20 08:02 supertanuki

Hi @Jolg42 , I've made at the end of 2019 several modifications about chrome extension support.

I watched your code, and he's a bit far from the current code from master branch. I think you should get the more recent from the branch and redo your modifications... so I'm here to help you 👍

I made several modifications from your work this afternoon, and it seems that I succeeded not only to merge your code, but also I made test alignement to maintain a acceptable test coverage.

Can you contact me directly to see how I can properly give you my code for a merge? I'm a bit rusted with Gitlab 👴

cguignol avatar Apr 13 '20 14:04 cguignol

Hi @cguignol 😄

I guess you can commit it directly and close this PR. That would be easier.

I'm curious to see the change, please send a link here if you can 😉

Jolg42 avatar Apr 13 '20 15:04 Jolg42

Hi @cguignol 😄

I guess you can commit it directly and close this PR. That would be easier.

I'm curious to see the change, please send a link here if you can 😉

I had to remove several "const" statements because it prevents the code to be tested (function call and mock problems). If you find a solution, be pleased to make the modifications.

I'll make the commit soon ;)

cguignol avatar Apr 14 '20 12:04 cguignol

Is there something preventing this one to be merged ? I'm also maintaining my own version locally to have more accurate results, I'd like to see it upstream so everyone can benefit from it

rakoo avatar May 13 '20 11:05 rakoo