Improvements in image upload functionality #3057

aydwi · 2018-02-21T23:40:28Z

Status

Ready for review

Description of Changes

This PR-

Fixes Admin logo image upload: Resize images server-side if they are too large #2807 by resizing the uploaded logo to a maximum width of 500px, or a maximum height of 450px (whichever is higher) while maintaining the original aspect ratio. Maximum limit is chosen in accordance with the recommended size by SecureDrop docs. Images with dimensions below this constraint are not resized.
Fixes Image uploaded by journalist is always saved as a PNG file #3043 by converting JPG/JPEG image files to PNG before saving them. Thus no matter which format the Admin chooses to upload, the final image is always a PNG file.
Possibly fixes Recommend dimensions for logo(s) uploads #2801 by adding a message at the logo upload screen informing the user about the recommended image size as per the docs.

Fixes Check for a valid image file when uploading logo #3058 by only accepting valid image files.

Dependencies Introduced

Python Pillow, installed via pip
libjpeg-dev, installed via apt

Testing

After successfully uploading, image size can be verified by checking the dimensions of securedrop/securedrop/static/i/custom_logo.png.
After successfully uploading a JPG/JPEG image, it can be verified that securedrop/securedrop/static/i/ always contains custom_logo.png
Journalist app should display the recommended size on the upload page.
Try to upload a non-image file with an extension like JPG/JPEG/PNG. A warning message should appear that image cannot be processed.

Checklist

If you made changes to the app code:

Unit and functional tests pass on the development VM

redshiftzero · 2018-02-22T00:53:09Z

This looks good on first inspection, note you have a few linting errors (once this is fixed then the full staging tests will run):

./securedrop/tests/test_journalist.py:753:80: E501 line too long (159 > 79 characters)
./securedrop/journalist_app/admin.py:41:50: E251 unexpected spaces around keyword / parameter equals
./securedrop/journalist_app/admin.py:41:52: E251 unexpected spaces around keyword / parameter equals

Note you can add # noqa after a line if you think there is a compelling reason it should not be linted

aydwi · 2018-02-22T00:59:06Z

I'll correct the linting issues. May I ask what does staging-test-with-rebase do?

conorsch · 2018-02-22T01:03:12Z

May I ask what does staging-test-with-rebase do?

The staging-test-with-rebase phase will:

Rebase the PR branch on top of the latest "develop" branch (thus the name!)
Provision "staging" Application and Monitor Servers
Run configuration tests against to validate server state
Run application tests to validate webapp functionality

We also had a staging-test phase, but rather than run both, we only run one. (See #2932 for recent changes.)

heartsucker · 2018-02-22T09:42:42Z

securedrop/journalist_templates/config.html

@@ -32,6 +32,9 @@ <h2>{{ gettext('Logo Image') }}</h2>
    {{ form.logo(id="logo-upload") }}
    <br>
  </p>
+  <h5>
+    {{gettext('Recommended size: 500px * 450px')}}


Two things here:

We use {{ gettext(...) }} (with spaces) stylistically to improve readability.

Is there a reason to suggest images that are 500x450 pixels but we thumbnail to 500x500 pixels max?

Thanks for letting me know. I'll adjust that.

Pillow suggests giving size as a 2-tuple. The thumbnail method automatically adjusts the lower dimension according to aspect ratio. 500 is the upper limit.

If a user uploads square image with edge length greater than 500px, the end result will be 500px by 500px image, since aspect ratio is preserved. It will be resized to 500px by 450px only when the initial aspect ratio was 10:9.

I think the size 500px by 450px comes from the size of default SecureDrop logo. Maybe we can recommend a square image like 500px by 500px.

I'm fine with either, but what we suggest and what we thumbnail to should be the same number just to avoid unexpected behavior.

I changed the maximum possible height of thumbnail to 450px. If default logo can be changed to 500 * 500, we can start recommending 500 * 500

heartsucker · 2018-02-22T09:43:41Z

securedrop/tests/test_journalist.py

            form = journalist_app.forms.LogoForm(
-                logo=(StringIO('imagedata'), 'test.png')
+                logo=(BytesIO(base64.decodestring
+                ("iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQ"


For the other reviewers, this was something I gave @aydwi out of band, and that is definitely a 1x1 white pixel.

Yes thanks @heartsucker. I added a comment to clarify that to anyone reading the code as well. Also, I think including an image whose size is greater than 500px can also help test the resize function, although the base64 string will be very large.

heartsucker · 2018-02-22T09:50:25Z

I would also change this:

class LogoForm(FlaskForm):
    logo = FileField(validators=[
        FileRequired(message=gettext('File required.')),
        FileAllowed(['jpg', 'png', 'jpeg'],
                    message=gettext('Upload images only.'))
    ])

To have the error message You can only upload files ending in .png, .jpg, or .jpeg to not confuse anyone if they try to upload an SVG or GIF since those are still images.

I know there's no ticket for this, but it would really make this PR better :D That said, this isn't a blocker on merging.

heartsucker · 2018-02-22T09:52:15Z

securedrop/journalist_app/admin.py

@@ -34,6 +36,11 @@ def manage_config():
            static_filepath = os.path.join(config.SECUREDROP_ROOT,


We should save this to a temp file, then once we've successfully resized it move it to the old location. Right now if we throw an error thumbnailing it, we'll have a broken image.

It should go:

Save to tmp

Thumbnail that overwrites tmp

Move tmp to static/i/logo.png using os.rename

Made this change

heartsucker · 2018-02-22T09:53:26Z

securedrop/journalist_app/admin.py

@@ -34,6 +36,11 @@ def manage_config():
            static_filepath = os.path.join(config.SECUREDROP_ROOT,
                                           "static/i/logo.png")
            f.save(static_filepath)
+
+            with Image.open(static_filepath) as im:


If someone gives us a PNG or JPG that Pillow doesn't understand, the use will get the generic 500 error page. We should try/catch this to and flash something like "We couldn't process that image. Try another one."

I did not write any exception handling here because I think Flask handles the type of images, and only JPG/JPEG/PNG make through. But that is a good point, Pillow may encounter something unexpected. I'll update the code.

I'm rather sure that the FileAllowed validator only checks the extension, and it's possible someone has a valid PNG that Pillow doesn't understand. We should try to catch these cases.

aydwi · 2018-02-22T10:21:41Z

Also, why is the lint failing again? 😟

heartsucker · 2018-02-22T10:28:28Z

It looks like we're having mypy errors, but this is all we get for the output.

make: Target 'lint' not remade because of errors.
if [[ ! -d .python3/.venv ]] ; then \
  virtualenv --python=python3 .python3/.venv && \
  .python3/.venv/bin/pip3 install mypy ; \
        fi
.python3/.venv/bin/mypy ./securedrop ./admin
Makefile:28: recipe for target 'ci-lint' failed
make: *** [ci-lint] Error 2
Exited with code 2

@kushaldas Some help here?

aydwi · 2018-02-22T10:50:02Z

@heartsucker If I change the message to 'You can only upload files ending in .png, .jpg, or .jpeg', should I also make changes to securedrop/translations directory?

ghost · 2018-02-22T11:02:11Z

The lint error is not the last line of the output because we intentionally run all lint checks even if one of them fail. The error at https://circleci.com/gh/freedomofpress/securedrop/8936?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link is:

flake8 --exclude='config.py,.venv/'
./securedrop/tests/test_journalist.py:754:17: E128 continuation line under-indented for visual indent
./securedrop/tests/test_journalist.py:755:17: E128 continuation line under-indented for visual indent

heartsucker · 2018-02-22T11:23:23Z

@aydwi No, you don't need to do that. That will be taken care of by a different process.

heartsucker

A few more notes.

Also, something we could consider to ease the logic of the logo change is to add an endpoint called sd_logo that does something lke

if path.exists(path.join(current_app.static_folder, 'i', 'custom_logo.png')):
    return redirect(url_for('static', filename='i/custom_logo.png'))
else:
    return redirect(url_for('static', filename='i/logo.png'))

This will give us a safe fallback in most error cases.

heartsucker · 2018-02-22T18:43:50Z

securedrop/journalist_app/admin.py

+                    imcopy = im.copy()
+                    imcopy.thumbnail((500, 450), resample=3)
+                    imcopy.save(temp_static_filepath, "PNG")
+                if os.path.exists(static_filepath):


I think this guard is unnecessary since we're already catching the IOError. Also if that path doesn't exist either because static/i/ is missing then we have big problems, or if static/i/logo.png is missing, then we should just copy it anyway.

or if static/i/logo.png is missing, then we should just copy it anyway.

I think I read once that the path to which we are moving should exist before os.rename() is called.

That wouldn't make sense for the API to do that because you'd always have to create a new file at the destination before doing a move. Also:

heartsucker@pythagoras:~$ mkdir /tmp/wat heartsucker@pythagoras:~$ cd !$ cd /tmp/wat heartsucker@pythagoras:/tmp/wat$ touch foo heartsucker@pythagoras:/tmp/wat$ python2 -c 'import os; os.rename("foo", "bar")' heartsucker@pythagoras:/tmp/wat$ ls bar

I'll remove the unnecessary guard then

heartsucker · 2018-02-22T18:45:19Z

securedrop/journalist_app/admin.py

+                    os.rename(temp_static_filepath, static_filepath)
+                flash(gettext("Image updated."), "logo-success")
+
+            except IOError:


I would change this to except Exception because I'm fairly sure that Image raises Pillow specific errors on all sorts of things, and we want to alert the user in all cases.

I'll update it

heartsucker · 2018-02-22T18:47:28Z

securedrop/journalist_app/admin.py

+                flash(gettext("Image updated."), "logo-success")
+
+            except IOError:
+                with open(static_filepath, 'w') as img_file:


This with block is unnecessary becuse os.rename is effectively atomic, so I think it's acceptable to assume that we never end up with a broken logo.png.

Since f.save(static_filepath) always writes logo.png, this with block replaces it with current image if uploaded image is broken. Without this restoration, only a message will be flashed, but the broken image will remain saved as logo.png

Yes, which is why I'm suggesting that we have an endpoint that selects and uses something called custom_logo.png so we don't end up with a broken logo.png. If the app terminates while the write in that with block happens, we'll have a broken logo. You're more IO than necessary, and that could be a problem. Also, the advantage of having and endpoint that selects custom_logo.png is that if someone runs the Ansible playbooks, they won't overwrite the uploaded image.

Before moving on to logo endpoint approach, I want to discuss my current approach and what purpose the two with blocks are serving.

Case 1-

User tries to upload logo for the first time
The image is a valid png file - A.png
The default SecureDrop logo is backed up
f.save writes A.png
In the try block, no exception occurs while working with image, images is resized and os.rename() executes
Thus the resized image is written at static_filepath

Case 2-

User takes a non-image file B.png (by renaming any other type of file to png) and tries to upload
The current logo is backed up by first with block
f.save writes B.png (which is a valid file, but not a valid image file)
Pillow raises IOError in the try block while trying to read the file, and os.rename() does not execute
In the except block, second with overwrites static_filepath with previously backed up image, which is not broken
Thus, the broken image is discarded, and a warning is shown

If the with blocks are removed, the non-image file will be saved. So I think they are not unnecessary in the current approach.

heartsucker · 2018-02-22T18:48:20Z

securedrop/journalist_app/admin.py

@@ -33,9 +35,30 @@ def manage_config():
            f = form.logo.data
            static_filepath = os.path.join(config.SECUREDROP_ROOT,
                                           "static/i/logo.png")
+            temp_static_filepath = os.path.join(config.SECUREDROP_ROOT,
+                                                "static/i/temp_logo.png")
+            with open(static_filepath) as img_file:


As noted in other comments, I don't think we need to read this in. os.rename should just do the right thing on its own.

heartsucker · 2018-02-22T18:49:58Z

securedrop/journalist_app/admin.py

@@ -33,9 +35,30 @@ def manage_config():
            f = form.logo.data
            static_filepath = os.path.join(config.SECUREDROP_ROOT,
                                           "static/i/logo.png")
+            temp_static_filepath = os.path.join(config.SECUREDROP_ROOT,
+                                                "static/i/temp_logo.png")


I'm not sure we should save temp files in ./static. I think we can use the SECUREDROP_DATA_ROOT and make sure there's a tmp in there. This would provide us with safe place for temp files.

aydwi · 2018-02-22T19:36:57Z

@heartsucker I'll try to incorporate these changes. Can I know why "staging-test-with-rebase" is failing?

msheiny · 2018-02-22T19:49:28Z

@aydwi its failing during app installation

No distributions at all found for pillow==5.0.0 (from -r /var/www/securedrop/requirements/securedrop-app-code-requirements.txt (line 20)

i don't think uppercase should matter but maybe pip interprets the requirements file differently than the cli tool? i dunno thats my first guess is to try setting it Pillow instead of pillow ?

aydwi · 2018-02-22T19:54:22Z

@msheiny I built the requirements file with pip-compile --output-file securedrop/requirements/securedrop-app-code-requirements.txt securedrop/requirements/securedrop-app-code-requirements.in.

I added Pillow to the .in file, but in .txt file it came as pillow. Is it a good approach to manually edit it?

msheiny · 2018-02-22T19:55:35Z

huh @aydwi .... that cant be it then.. if pip-tools is handling it, should be fine.... hrmmm

heartsucker · 2018-02-22T20:39:14Z

@msheiny @aydwi

heartsucker@pythagoras:/tmp$ virtualenv -p `which python2` venv
Running virtualenv with interpreter /usr/bin/python2.7
Using base prefix '/usr'
New python executable in /tmp/venv/bin/python2.7
Also creating executable in /tmp/venv/bin/python
Installing setuptools, pip, wheel... done.
heartsucker@pythagoras:/tmp$ source venv/bin/activate
(venv) heartsucker@pythagoras:/tmp$ pip install pillow==5.0.0
Collecting pillow==5.0.0
  Using cached Pillow-5.0.0-cp35-cp35m-manylinux1_x86_64.whl
Installing collected packages: pillow
Successfully installed pillow-5.0.0

ghost · 2018-02-23T11:07:10Z

https://circleci.com/gh/freedomofpress/securedrop/8969?utm_campaign=vcs-integration-link&utm_medium=referral&utm_source=github-build-link shows in the Run Debian builds part which is green / success:

    TASK [build-securedrop-app-code-deb-pkg : Create pip wheel archive for Debian package requirements.] 
    changed: [build]
    
    TASK [build-securedrop-app-code-deb-pkg : Install dependencies for building translations] ***
    changed: [build] => (item=[u'gnupg2', u'haveged', u'python', u'python-pip', u'secure-delete', u'sqlite', u'apparmor-utils', u'redis-server', u'supervisor', u'libjpeg-dev'])

But Create pip wheel archive for Debian package does

- name: Create pip wheel archive for Debian package requirements.
  command: >
    pip wheel
    -r {{ securedrop_code_filtered }}/requirements/securedrop-app-code-requirements.txt
    -w {{ securedrop_app_code_deb_dir }}/var/securedrop/wheelhouse
  tags: pip

which should fail to populate the wheelhouse with pillow ... because the dependency it needs (libjpeg-dev) is installed after. So it looks like pip wheel does not fail when it should and we have wheelhouse that does not contain pillow, reason why it fails to install from the package (because the package does not download from the net, it installs from locally available wheelhouse exclusively).

ghost · 2018-02-23T11:24:42Z

I tried manually and failed to reproduce with

$ cd securedrop
$ ./bin/dev-shell bash
$ sudo bash
# apt-get remove --purge libjpeg-dev
# pip uninstall pillow
# pip wheel -r requirements/securedrop-app-code-requirements.txt -w /tmp/w
# ls /tmp/w/Pillow*
Pillow-5.0.0-cp27-none-linux_x86_64.whl

Trying make build-debs now to verify that Pillow is misisng from the wheelhouse. If it is not missing I'll rebase & repush to confirm.

ghost · 2018-02-23T11:47:37Z

you need to add libjpeg-dev in molecule/builder/Dockerfile which is where the package is built and the wheelhouse populated (the development environment spawned with make -C securedrop dev is different, reason why it succeeds above). Without libjpeg-dev the build fails with:

The headers or library files could not be found for jpeg,

a required dependency when compiling Pillow from source.



Please see the install instructions at:

   https://pillow.readthedocs.io/en/latest/installation.html



Traceback (most recent call last):

  File "", line 1, in 

  File "/tmp/pip_build_root/pillow/setup.py", line 804, in 

    raise RequiredDependencyException(msg)

__main__.RequiredDependencyException:



The headers or library files could not be found for jpeg,

a required dependency when compiling Pillow from source.



Please see the install instructions at:

   https://pillow.readthedocs.io/en/latest/installation.html





----------------------------------------
  Failed building wheel for pillow
  Running setup.py bdist_wheel for psutil
  Destination directory: /tmp/z
  Running setup.py bdist_wheel for pycryptodomex
  Destination directory: /tmp/z
  Running setup.py bdist_wheel for scrypt
  Destination directory: /tmp/z
  Running setup.py bdist_wheel for sqlalchemy
  Destination directory: /tmp/z
  Running setup.py bdist_wheel for webassets
  Destination directory: /tmp/z
  Running setup.py bdist_wheel for wtforms
  Destination directory: /tmp/z
Successfully built cssmin flask-assets gnupg itsdangerous jsmin markupsafe psutil pycryptodomex scrypt sqlalchemy webassets wtforms
Failed to build pillow

but it does not trigger an error because pip wheel never returns on error when it fails, it just outputs and error message.

aydwi · 2018-02-23T11:54:21Z

@dachary Okay

ghost · 2018-02-23T12:03:41Z

For the record #3065 will fail the build in a more debuggable way in the future.

aydwi · 2018-02-24T10:26:12Z

@heartsucker We wouldn't want to revert to in-place resizing of logo. Is there a way permissions can be granted? Anyways I'm working on a way to handle this, will share soon.

heartsucker · 2018-02-24T15:11:06Z

@aydwi In chat, @dachary corrected me that I have it backwards. The logo needs to be owned by www-data, and during tests it ends up owned by root. I also said that I think we shouldn't be overwriting the original logo.png. We should add an endpoint that:

checks for the existence of a file under SECUREDROP_DATA_ROOT/admin/logo.png and serves that
OR if it's missing serves the original static/i/logo.png

When we do this, we don't have to ever touch the original logo.png. This will unbreak the testinfra test and also have our code be more robust against failures.

aydwi · 2018-02-24T16:34:42Z

I'm working on it. Before that, I'll push another method for review.

codecov-io · 2018-02-24T17:45:11Z

Codecov Report

Merging #3057 into develop will decrease coverage by 2.24%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #3057      +/-   ##
===========================================
- Coverage    86.38%   84.13%   -2.25%     
===========================================
  Files           34       34              
  Lines         2122     2042      -80     
  Branches       233      223      -10     
===========================================
- Hits          1833     1718     -115     
- Misses         234      271      +37     
+ Partials        55       53       -2

Impacted Files	Coverage Δ
i18n_tool.py	`67.03% <0%> (-29.13%)`	⬇️
journalist_app/__init__.py	`83.75% <0%> (-5%)`	⬇️
source_app/main.py	`95.74% <0%> (-2.09%)`	⬇️
journalist_app/main.py	`72.44% <0%> (-1.75%)`	⬇️
journalist_app/admin.py	`87.58% <0%> (-0.86%)`	⬇️
source_app/utils.py	`85.71% <0%> (-0.26%)`	⬇️
crypto_util.py	`95.91% <0%> (-0.05%)`	⬇️
secure_tempfile.py	`100% <0%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 46aa4c0...8372f43. Read the comment docs.

ghost · 2018-02-26T22:15:12Z

In case you don't know about it @aydwi github does not notify when you push new commits. You should explicitly ping @heartsucker and let him know you have something new addressing his comments. If you already knew just ignore me :-P

heartsucker · 2018-02-27T15:34:13Z

securedrop/journalist_app/admin.py

            try:
-                with Image.open(static_filepath) as im:
+                with Image.open(f) as im:


I'm not sure why you're copying the image and than saving the copy. The original is still being overwritten so we can still end up in a place where we have a broken image.

I was trying to work with a copy of image instead of modifying it in place. That is unnecessary though, I'll remove it.

ghost · 2018-03-12T15:02:23Z

@aydwi a gentle reminder about the need to rebase this pull request

aydwi · 2018-03-12T15:07:33Z

@dachary Sure. I'll rebase it with develop 🙂

aydwi · 2018-03-14T22:40:10Z

@heartsucker Pushed the changes to implement the endpoint and squashed the commits

ghost · 2018-03-15T11:24:14Z

@aydwi you need another rebase because changes to tests were merged in the past 24h. Good practice :-)

aydwi · 2018-03-15T11:26:00Z

@dachary Will do

Added blank lines Fix linting issues Made requested changes Linting build: pip wheel must fail when it does not build a python module build: re-create the builder docker image when Dockerfile changes It takes longer but works all the time. Uploading an image to a docker repository to speed-up the build requires an additional manual step and access to the docker repository. A possible optimization (would be a few minutes faster) could be implemented by storing docker layers, similar to what is done for the Dockerfile used when testing. Try to fix failing CI build Changes in methodology Added an endpoint to handle custom logo image Add logo endpoint to _insecure_views Fix the error in the previous commit

kushaldas · 2018-03-28T10:14:05Z

@aydwi you should also clean up the commit message in this PR.

aydwi · 2018-03-28T10:16:10Z

@kushaldas Sure

heartsucker · 2018-04-03T13:18:25Z

I'm going to merge this because it is a strict improvement over the current behavior. However, I am going to open up some followup tickets to tighten this up a little bit (for example, if we want to save in /var/lib/securedrop instead of static because then we get slightly nicer automatic cache/busting behavior).

aydwi requested a review from conorsch as a code owner February 21, 2018 23:40

aydwi requested a review from a user February 21, 2018 23:40

aydwi requested review from heartsucker, msheiny and redshiftzero as code owners February 21, 2018 23:40

ghost added UX goals: journalist experience labels Feb 22, 2018

heartsucker reviewed Feb 22, 2018

View reviewed changes

heartsucker suggested changes Feb 22, 2018

View reviewed changes

aydwi mentioned this pull request Feb 22, 2018

Check for a valid image file when uploading logo #3058

Closed

heartsucker suggested changes Feb 22, 2018

View reviewed changes

heartsucker reviewed Feb 27, 2018

View reviewed changes

heartsucker approved these changes Apr 3, 2018

View reviewed changes

heartsucker merged commit 9880a30 into freedomofpress:develop Apr 3, 2018

This was referenced Apr 3, 2018

Save custom logos to /var/lib instead of Flask.static_folder #3219

Open

AppArmor doesn't allow apache to read/write custom logo #3221

Closed

eloquence added this to the 0.7 milestone Apr 5, 2018

This was referenced May 7, 2018

Remove functionality requiring libjpeg-dev #3375

Closed

cron-apt update logic only allows addition of apt dependencies from /etc/apt/security.list #3376

Closed

heartsucker mentioned this pull request Jun 15, 2018

[0.8.0] Custom Logo does not persist with upgrade #3528

Closed

eloquence mentioned this pull request Jun 27, 2020

Remove CUSTOM_HEADER_IMAGE legacy code #5347

Closed

eloquence mentioned this pull request Mar 5, 2021

Do not treat logo image as a conffile in app code package #5850

Closed

eloquence mentioned this pull request Mar 13, 2021

Backup script does not include custom logo #5868

Closed

		@@ -34,6 +36,11 @@ def manage_config():
		static_filepath = os.path.join(config.SECUREDROP_ROOT,

Improvements in image upload functionality #3057

Improvements in image upload functionality #3057

Conversation

aydwi commented Feb 21, 2018 • edited Loading

Status

Description of Changes

Dependencies Introduced

Testing

Checklist

If you made changes to the app code:

redshiftzero commented Feb 22, 2018

aydwi commented Feb 22, 2018

conorsch commented Feb 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aydwi Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aydwi Feb 22, 2018 • edited Loading

Choose a reason for hiding this comment

heartsucker commented Feb 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aydwi commented Feb 22, 2018

heartsucker commented Feb 22, 2018

aydwi commented Feb 22, 2018

ghost commented Feb 22, 2018

heartsucker commented Feb 22, 2018

heartsucker left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aydwi Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aydwi Feb 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

aydwi commented Feb 22, 2018

msheiny commented Feb 22, 2018

aydwi commented Feb 22, 2018 • edited Loading

msheiny commented Feb 22, 2018

heartsucker commented Feb 22, 2018 • edited Loading

ghost commented Feb 23, 2018 • edited by ghost Loading

ghost commented Feb 23, 2018

ghost commented Feb 23, 2018 • edited by ghost Loading

aydwi commented Feb 23, 2018

ghost commented Feb 23, 2018

aydwi commented Feb 24, 2018 • edited Loading

heartsucker commented Feb 24, 2018

aydwi commented Feb 24, 2018 • edited Loading

codecov-io commented Feb 24, 2018 • edited Loading

Codecov Report

ghost commented Feb 26, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ghost commented Mar 12, 2018

aydwi commented Mar 12, 2018

aydwi commented Mar 14, 2018 • edited Loading

ghost commented Mar 15, 2018

aydwi commented Mar 15, 2018

kushaldas commented Mar 28, 2018

aydwi commented Mar 28, 2018

heartsucker commented Apr 3, 2018

aydwi commented Feb 21, 2018 •

edited

Loading

aydwi Feb 22, 2018 •

edited

Loading

aydwi Feb 22, 2018 •

edited

Loading

aydwi Feb 23, 2018 •

edited

Loading

aydwi Feb 23, 2018 •

edited

Loading

aydwi commented Feb 22, 2018 •

edited

Loading

heartsucker commented Feb 22, 2018 •

edited

Loading

ghost commented Feb 23, 2018 •

edited by ghost

Loading

ghost commented Feb 23, 2018 •

edited by ghost

Loading

aydwi commented Feb 24, 2018 •

edited

Loading

aydwi commented Feb 24, 2018 •

edited

Loading

codecov-io commented Feb 24, 2018 •

edited

Loading

aydwi commented Mar 14, 2018 •

edited

Loading