Add user settings module #143

lukpueh · 2017-10-26T20:47:27Z

The newly added module provides methods to read environment variables and rcfiles and to write them to the in_toto.settings module, i.e. override them (fix #137).

The PR also adds a call of the settings parsing and overriding method to in-toto's __init__.py, which should be the first thing that's executed when using in_toto.
Any code that accesses in_toto.settings thereafter receives the values defined by the user.

More information about how the user sets the settings can be found in the header docstring of user_settings.py.

coveralls · 2017-10-26T20:50:29Z

Coverage increased (+0.02%) to 99.433% when pulling 1832e97 on user-settings into 7b6eaf2 on develop.

vladimir-v-diaz

My comments for the main modules, I still need to review test_user_settings.py

vladimir-v-diaz · 2017-11-01T14:47:20Z

in_toto/__init__.py

@@ -0,0 +1,3 @@
+from in_toto.user_settings import set_settings


Our code style guidelines advises against importing items from a module: https://github.com/secure-systems-lab/code-style-guidelines#imports

What's wrong with the following?

import in_toto.user_settings in_toto.user_settings_setting.set_settings()

vladimir-v-diaz · 2017-11-01T14:47:43Z

in_toto/__init__.py

@@ -0,0 +1,3 @@
+from in_toto.user_settings import set_settings
+# Override settings in settings.py


Can you put some blank lines between import statements and the body of the module?

vladimir-v-diaz · 2017-11-01T14:52:49Z

in_toto/__init__.py

@@ -0,0 +1,3 @@
+from in_toto.user_settings import set_settings
+# Override settings in settings.py
+set_settings()


It seems wrong to perform actions when a project or module is imported.

Also, are you careful to catch exceptions or errors when your project is imported? It would be strange to get an error about a missing or corrupted config file when attempting to import the project.

Good point, I actually had a similar feeling. It's probably better to only monkey patch the settings module when using a command line tool. What do you think?

Yes, it makes more sense to me to have the command-line tool read the config file(s) and environment variables. The command tool help message or its documentation should explain how the settings can be modified via the environment variables and config files. It should also list the supported settings.

vladimir-v-diaz · 2017-11-01T14:59:29Z

in_toto/user_settings.py

+  them to the in_toto.settings module (override them).
+
+  Notes:
+  - Variable values are converted to lists if they contain colons


Should this be "environment values", so as not to confuse it with a Python variable?

Values form envvars or rcfiles. I will rephrase.

vladimir-v-diaz · 2017-11-01T15:08:28Z

in_toto/user_settings.py

+
+  Notes:
+  - Variable values are converted to lists if they contain colons
+  - Variable names are converted to upper case


To be precise, do you mean that in-toto settings are always upper case?

Can you be a bit more precise here, perhaps providing an example? Are in-toto.settings.SOME_SETTING settings always upper case regardless of the way that environments variables are written (i.e., upper case, lower case, or mixed case) ?

vladimir-v-diaz · 2017-11-01T15:31:31Z

in_toto/user_settings.py

+ENV_PREFIX = "IN_TOTO_"
+USER = os.path.expanduser("~")
+
+RC_PATHS = [


How come this list doesn't include the .in_totorc RC file in the current working directory? See line 40.

This list does include .in_totorc. See your comment below.

vladimir-v-diaz · 2017-11-01T15:35:04Z

in_toto/user_settings.py

+
+
+def get_rc():
+  """Load RCfiles from the usual places and return a dictionary of


The usual places? Can you be more explicit here? How about, "the paths specified in RC_PATHS?

Preferably, you should assume that a developer/reader is only looking at this docstring and is unaware of the module's other functions and docstrings.

vladimir-v-diaz · 2017-11-01T15:40:36Z

in_toto/user_settings.py

+  return env_dict
+
+
+def get_rc():


How come this function's docstring isn't formatted according to our code style guidelines? Where is the section on exceptions, return values, purpose, etc.?

The other functions in this module are also missing proper docstrings.

I thought that we only use this convention for API functions. But you are right, it says
"for every function [..] that will be called from other modules".

I'll change this.

vladimir-v-diaz · 2017-11-01T15:57:21Z

in_toto/user_settings.py

+  user_settings.update(get_rc())
+
+  for name, value in user_settings.iteritems():
+    setattr(SETTINGS_MODULE, name, value)


Is it possible to populate in-toto.settings with unexpected variables or values? For example, in-toto.settings.[] How do you expect the module to catch these exceptions or fail in these instances?

vladimir-v-diaz · 2017-11-01T16:00:17Z

test/test_user_settings.py

+  See LICENSE for licensing information.
+
+<Purpose>
+  Test user settings module.


I find the following comment easier to determine which module is being tested:

Test in-toto.user_settings.py.

vladimir-v-diaz

Okay, here are my remaining comments for this pull request.

vladimir-v-diaz · 2017-11-01T17:23:33Z

in_toto/user_settings.py

+  return rc_dict
+
+
+def set_settings():


Is it wise to first backup the current settings before attempting to overwrite them?

Not unless we use the backup for something (e.g. rollback on error). But considering that we use a whitelist, which reduces the unexpected error likelihood; and only use this function in cli tools, where I'd rather exit on error, I think we don't need to backup.

vladimir-v-diaz · 2017-11-01T17:38:01Z

test/test_user_settings.py

+
+    os.environ["IN_TOTO_ARTIFACT_EXCLUDES"] = "e:n:v"
+    os.environ["IN_TOTO_artifact_basepath"] = "e/n/v"
+    os.environ["IN_TOTO_NEW_ENV_SETTING"] = "new env setting"


Is this manually setting some environment variables? Is it possible to temporarily set some environment variables and verify that environment variables are properly loaded?

Perhaps testing actual environment variables on the system is unnecessary, but something to consider.

I thought about it, but that would actually be a test for os.environ.

vladimir-v-diaz · 2017-11-01T17:41:17Z

test/test_user_settings.py

+import shutil
+import tempfile
+import in_toto.settings
+from in_toto.user_settings import set_settings, get_env, get_rc


I guess it's okay to import individual items from a module (in this unit test), but you should get into the habit of using absolute import statements.

in-toto.user_settings.get_rc()

set_settings()

The former is more readable in my opinion, since it shows where the function comes from. The latter can be confused for some function in this module.

vladimir-v-diaz · 2017-11-01T17:42:54Z

test/test_user_settings.py

+    for key in dir(in_toto.settings):
+      self.settings_backup[key] = getattr(in_toto.settings, key)
+
+    # Create an rc file and set some environment variables


Another thing you can do is to have a pre-generated .in_totorc that you can load in the unit tests. This approach might be faster, easier for others to inspect and review, etc.

vladimir-v-diaz · 2017-11-01T17:45:14Z

test/test_user_settings.py

+
+  @classmethod
+  def tearDownClass(self):
+    os.chdir(self.working_dir)


If you use pre-generated config files, another thing it free you from is the need to change directories and clean up temp files. The unit tests would only need to load config files.

vladimir-v-diaz · 2017-11-01T17:53:16Z

test/test_user_settings.py

+          "ARTIFACT_EXCLUDES=r:c:file\n"
+          "new_rc_setting=new rc setting")
+
+    os.environ["IN_TOTO_ARTIFACT_EXCLUDES"] = "e:n:v"


Can you test a value that contains special characters like $, %, .., etc.? What other interesting things? Python identifiers? Function calls?

I don't evaluate the strings. So it should be fine.

Is is possible for someone to attempt a import in-toto.settings and get errors because of unexpected entries?

I added sanitization for the available settings where used in 4072cb3.

vladimir-v-diaz · 2017-11-01T18:04:30Z

in_toto/user_settings.py

+  user_settings.update(get_rc())
+
+  for name, value in user_settings.iteritems():
+    setattr(SETTINGS_MODULE, name, value)


Can you verify that someone can't cause unintended lines? Is it possible to populate in-toto.settings.py with malicious input like: "OS.REMOVE(my_files)".lower()

Perhaps you don't have to worry about malicious input in config files and environment variables, but something to ponder.

It does not seem possible, since I am assigning module variables with strings.

However, there isn't really a need to allow everything, so it might be better to make a whitelist, i.e. only allow settings for which there is a default in settings.py.

Using a whitelist is the more secure approach in my opinion.

vladimir-v-diaz · 2017-11-01T18:09:07Z

test/test_user_settings.py

+    self.assertFalse("NOT_IN_TOTO_NOTHING" in env_dict)
+
+
+  def test_set_settings(self):


Is it possible to test the import behavior that follows from the modified __init__()? These test conditions appear to test environment and RCfiles indirectly.

vladimir-v-diaz · 2017-11-01T18:14:33Z

test/test_user_settings.py

+    self.assertEquals(in_toto.settings.NEW_RC_SETTING, "new rc setting")
+
+    # Settings that are only in env are still used even if they are new
+    self.assertEquals(in_toto.settings.NEW_ENV_SETTING, "new env setting")


Should you have a test condition that verifies overwritten settings? Suppose in-toto.settings.MY_SETTING is already set, and you attempt to call set_settings(). Is MY_SETTING overwritten, as expected?

ARTIFACT_BASEPATH and ARTIFACT_EXCLUDES have defaults in settings.py. So the overriding is already tested.

I see. Should the test and/or modules indicate that ARTIFACT_BASEPATH and ARTIFACT_EXCLUDES are actual in-toto settings? If you happen to remove these settings in the future, do these tests conditions become invalid (in terms of failing to test for the overwriting behavior)? It's good to note assumptions in the comments for these sort of situations

Also, assuming that user_settings.py overwrites settings in in-toto.settings.py, wouldn't a more accurate test (1) verify that the new setting is an actual supported setting in in-toto.settings.py (2) note the value of the setting and (3) verify that the value in (2) is overwritten with the new value?

vladimir-v-diaz · 2017-11-01T18:17:35Z

in_toto/user_settings.py

+"""
+import os
+import ConfigParser
+import in_toto.settings as SETTINGS_MODULE


Is there a reason why you use import X as FOO? In this case,
SETTINGS_MODULE is only one character longer, and less readable than in_toto.settings.

@vladimir-v-diaz

As pointed out by @vladimir-v-diaz in #143, our style guidelines advice against importing indiviudal items from modules.

coveralls · 2017-11-01T23:00:39Z

Coverage increased (+0.02%) to 99.433% when pulling 728fd84 on user-settings into 7b6eaf2 on develop.

coveralls · 2017-11-02T18:15:11Z

Coverage increased (+0.02%) to 99.434% when pulling 7fc91ad on user-settings into 7b6eaf2 on develop.

lukpueh · 2017-11-02T22:13:28Z

@vladimir-v-diaz, thanks for you comments. I tried to address them accordingly. Let me know what you think.

Update: It seems that 2ea0e5d broke it, because ConfigParser converts names to lower case. I'll fix this on Monday. You can review the rest if you want though.

@vladimir-v-diaz

Updates user_settings unittests following @vladimir-v-diaz's comments in #143 and fixes typos in user_settings docstrings.

coveralls · 2017-11-06T21:56:17Z

Coverage increased (+0.02%) to 99.438% when pulling eecac16 on user-settings into 7b6eaf2 on develop.

lukpueh · 2017-11-06T21:56:32Z

I fixed the remaining issues regarding case sensitivity that broke the unittests last week, added some testcases and fixed missing settings sanitization where they are used. Mind giving it another pass, @vladimir-v-diaz?

vladimir-v-diaz · 2017-11-07T17:49:42Z

README.md

+these settings can be configured using environment variables or RCfiles.
+Take a look at [`in_toto.settings`](https://github.com/in-toto/in-toto/blob/develop/in_toto/settings.py)
+to learn more about the available defaults and at
+[`in_toto.user_settings`](https://github.com/in-toto/in-toto/blob/develop/in_toto/user_settings.py)


I get a 404 error with this link.

I'm not so sure about the documentation of this section. Is the command-line tool intended to be run by a variety of users? If so, wouldn't linking to a Python module might be inappropriate for these users?

Re 404: should have used relative path (user_settings.py isn't in the main branch yet).
Re inappropriate: Agreed, will change.

This module provides methods to read environment variables and rcfiles and to write them to the in_toto.settings module, i.e. override them. The commit also adds a call of the settings parsing and overriding method to in-toto's __init__.py, which should be the first thing that's executed when using in_toto. Any code that accesses in_toto.settings thereafter receives the values defined by the user.

@vladimir-v-diaz

As pointed out by @vladimir-v-diaz in #143, our style guidelines advice against importing indiviudal items from modules.

This commit minimizes the test setup/teardown by adding a pre- generated rcfile instead of writing a temp dir and an rcfile on the fly, which should make the test slightly faster and maybe more readable.

There is no need for unlimited monkeypatching of the settings module using envvars and rcfiles, since in-toto only uses a known set of settings anyway. Moreover, a whitelist approach seems more secure. This commit also updates the relevant unittest, renames a variable and extends some comments and docstrings.

This commit moves the test `.in_totorc` file from the `demo_files` dir to a new `rc_test` dir. Other unit tests copy all files from `demo_files` to their cwd, and would therefor load the settings from `.in_totorc` although they rely on the defaults in `settings.py`. This commit also removes the default settings backup and restore in test_user_settings.py's setup/teardown methods, because they are not needed.

@vladimir-v-diaz

As pointed out by @vladimir-v-diaz it is odd to perform actions like monkeypatching a module using env variables and rcfiles when importing a package or module. This behavior is better suited when the package is used via command line tools. Hence, this commit removes the a call to `user_settings.set_settings` from __init__ and instead calls the function in the command line interfaces.

Call log.info if a setting is overridden

User settings (envvars and rcfiles) only allow strings, but the setting `LOG_LEVEL` expects a constant from the Python `logging` module. Since `LOG_LEVEL` is likely to be removed (#117) we just ignore it for now and remove it from the user settings whitelist.

The case of settings defined in envvars or rcfiles is no longer ignored, that is the currently whitelisted settings must be defined as such: ARTIFACT_BASE_PATH and ARTIFACT_EXCLUDES in rcfiles and IN_TOTO_ARTIFACT_BASE_PATH and IN_TOTO_ARTIFACT_EXCLUDES in environment variables. Since the user settings module monkey patches (whitelisted) settings it feels more secure to be more restrictive, e.g. by also requiring the case to match.

@vladimir-v-diaz

Following @vladimir-v-diaz's advice this commit updates the docstrings in the user_settings module. Details can be found in the function docstrings, which conform with our style guidelines. The header gives only an overview and references the respective functions.

Adds "reset" of `optionxform` default conversion to enable case sensitivity in rcfile optiosn parsed with Python's `ConfigParser`. https://docs.python.org/2/library/configparser.html#ConfigParser.RawConfigParser.optionxform

Both of the currently available settings are used in `runlib.record_artifacts_as_dict`. This commits adds a new Exception subclass `in_toto.exceptions.SettingsError` and updates the runlib function to raise this error if any of the two settings is invalid.

@vladimir-v-diaz

Updates user_settings unittests following @vladimir-v-diaz's comments in #143 and fixes typos in user_settings docstrings.

coveralls · 2017-11-07T18:49:34Z

Coverage increased (+0.02%) to 99.431% when pulling 3f686d5 on user-settings into 57d6a62 on develop.

vladimir-v-diaz

Here is the requested review of your follow-on commits. I had minor comments. Feel free to merge when you feel the pull request is ready.

vladimir-v-diaz · 2017-11-07T20:13:24Z

in_toto/runlib.py

@@ -119,7 +120,7 @@ def record_artifacts_as_dict(artifacts):
            the link command.

  <Exceptions>
-    None.
+    SettingsError if ARTIFACT_BASE_PATH or ARTIFACT_EXCLUDES can't be used


It might be better if this listed the exception name: in-toto.exceptions.SettingsError, since the caller will have to specify the full name to correctly catch the exception.

vladimir-v-diaz · 2017-11-07T20:24:22Z

in_toto/runlib.py

@@ -137,18 +138,29 @@ def record_artifacts_as_dict(artifacts):
    original_cwd = os.getcwd()
    try:
      os.chdir(in_toto.settings.ARTIFACT_BASE_PATH)
-    except OSError as e:
-      raise OSError("Review your ARTIFACT_BASE_PATH setting - {}".format(e))
+    except Exception as e:


Is there a reason why you switched from catching OSError to catching Exception? According to the Python documentation, all functions in the os modules raise OSError

in the case of invalid or inaccessible file names and paths, or other arguments that have the correct type, but are not accepted by the operating system.

It could also be a TypeError. I figured I just catch all Exceptions and re-raise as SettingsError.

You can also explicitly catch all the exceptions that you're aware of. For the unexpected exceptions, it might be better to let the exception pass through.

try: os.chdir(...) except (OSError, TypeError) as e: ...

A problem I have encountered in the past is that the code would sometimes catch weird things like NameError and the exception message would be totally wrong. You append the original exception, so I guess it would be easy to debug / catch these cases.

vladimir-v-diaz · 2017-11-07T20:25:07Z

in_toto/runlib.py


  # Normalize passed paths
  norm_artifacts = []
  for path in artifacts:
    norm_artifacts.append(os.path.normpath(path))

+  # If ARTIFACT_EXCLUDES is set it must be a list of strings or an empty


Or an empty what?

vladimir-v-diaz · 2017-11-07T20:29:07Z

in_toto/runlib.py

-      raise OSError("Review your ARTIFACT_BASE_PATH setting - {}".format(e))
+    except Exception as e:
+      raise in_toto.exceptions.SettingsError(
+          "Review your ARTIFACT_BASE_PATH setting '{}' - {}".format(


Consider providing more information on what could have gone wrong. For example, an invalid path. On the other hand, appending the exception, as you have done, would make it clear what is wrong with ARTIFACT_BASE_PATH.

I think adding the exception message is good enough. I guess we can always enhance this later, e.g. when taking care of #117.

vladimir-v-diaz · 2017-11-07T20:36:21Z

in_toto/runlib.py

+  # If ARTIFACT_EXCLUDES is set it must be a list of strings or an empty
+  # TODO: Change NAMES_SCHEMA to something more semantically accurate
+  if (in_toto.settings.ARTIFACT_EXCLUDES and not
+      securesystemslib.formats.NAMES_SCHEMA.matches(


Maybe PATH_SCHEMA or PATHS_SCHEMA is a bit more accurate? I am not certain, but ARTIFACT_EXCLUDES appears to be glob patterns?

vladimir-v-diaz · 2017-11-07T20:49:19Z

in_toto/user_settings.py

+    converted into a list.
+
+
+    Example:


vladimir-v-diaz · 2017-11-07T20:51:37Z

in_toto/user_settings.py

+    None.
+
+  <Side Effects>
+    Calls function to read files from disk.


For a while I used to list similar entries in Side Effects, but others have pointed out that this isn't really a side effect.

I remember having this discussion and also that there were uncertainties about some things and whether they would qualify as side effect. But I am pretty sure that there was no doubt about I/O.

vladimir-v-diaz · 2017-11-07T20:53:55Z

in_toto/user_settings.py

+    None.
+
+  <Side Effects>
+    Calls function to read files from disk.


Another instance where someone might argue that this isn't really a side effect.

vladimir-v-diaz · 2017-11-07T20:56:46Z

in_toto/user_settings.py

+      log.info("Using setting {0}={1}".format(
+          setting, user_setting))
+      setattr(in_toto.settings, setting, user_setting)


Consider adding an else-statement that logs a debug message for settings that are ignored/invalid.

Adding that else-statement would actually log the default settings and not the ignored/invalid settings, which might be more interesting anyway. I will add this.

vladimir-v-diaz · 2017-11-07T21:00:46Z

test/test_user_settings.py

-          "artifact_basepath=r/c/file\n"
-          "ARTIFACT_EXCLUDES=r:c:file\n"
-          "new_rc_setting=new rc setting")
+    # We use `demo_files` as test dir because it has an `.in_totorc`, which


I don't understand this comment. Is demo_files supposed to be rc_test?

Oh, I forgot to update this comment. I first added the rcfile to demo_files, as you suggested, but then discovered that this would break other tests that copied everything from demo_files and therefor also loaded and used the test rcfile.

coveralls · 2017-11-07T21:42:42Z

Coverage increased (+0.02%) to 99.431% when pulling 44f95ad on user-settings into 57d6a62 on develop.

@vladimir-v-diaz

Addressing @vladimir-v-diaz's review regarding comments and log statements related to user settings.

Since parsing the user settings has verbose output, it is better to postpone it until after parsing command line tool arguments, based on which e.g. the verbosity level of the program is set.

coveralls · 2017-11-07T22:37:45Z

Coverage increased (+0.02%) to 99.432% when pulling dffa53a on user-settings into 57d6a62 on develop.

The setting lists patterns to exclude artifacts, hence the new better suited name: ARTIFACT_EXCLUDE_PATTERNS

coveralls · 2017-11-07T22:44:33Z

Coverage increased (+0.02%) to 99.432% when pulling 3624c53 on user-settings into 57d6a62 on develop.

vladimir-v-diaz · 2017-11-07T23:03:13Z

README.md

@@ -122,7 +122,7 @@ Also, setting values that contain colons are parsed as list.

 ##### Available Settings

-`ARTIFACT_EXCLUDES` Specifies a list of glob patterns that can be used to
+`ARTIFACT_EXCLUDE_PATTERNS` Specifies a list of glob patterns that can be used to


vladimir-v-diaz · 2017-11-07T23:03:54Z

LBTM (Looks Brilliant To Me)

:)

vladimir-v-diaz self-requested a review November 1, 2017 14:12

vladimir-v-diaz reviewed Nov 1, 2017

View reviewed changes

lukpueh added a commit that referenced this pull request Nov 1, 2017

Replace from imports related to user settings

198eae9

As pointed out by @vladimir-v-diaz in #143, our style guidelines advice against importing indiviudal items from modules.

lukpueh force-pushed the user-settings branch from 072097c to 8152af4 Compare November 2, 2017 22:33

lukpueh added a commit that referenced this pull request Nov 6, 2017

Update user_settings tests + doc typo fixes

eecac16

Updates user_settings unittests following @vladimir-v-diaz's comments in #143 and fixes typos in user_settings docstrings.

vladimir-v-diaz reviewed Nov 7, 2017

View reviewed changes

lukpueh added 17 commits November 7, 2017 13:03

Add unit tests for user settings module

7046a26

Replace from imports related to user settings

2f7572d

As pointed out by @vladimir-v-diaz in #143, our style guidelines advice against importing indiviudal items from modules.

Rephrase purpose of test_user_settings.py

9d72216

Use pre-generated rcfile for user_settings tests

9471cda

This commit minimizes the test setup/teardown by adding a pre- generated rcfile instead of writing a temp dir and an rcfile on the fly, which should make the test slightly faster and maybe more readable.

Add logger to user_settings module

3fcf3f4

Call log.info if a setting is overridden

Update header and comments in settings.py

d95a11a

Add section about (user) settings to README

2719993

Enable case-sensitivity for rcfile options

1bf441e

Adds "reset" of `optionxform` default conversion to enable case sensitivity in rcfile optiosn parsed with Python's `ConfigParser`. https://docs.python.org/2/library/configparser.html#ConfigParser.RawConfigParser.optionxform

Update user_settings tests + doc typo fixes

3f686d5

Updates user_settings unittests following @vladimir-v-diaz's comments in #143 and fixes typos in user_settings docstrings.

lukpueh force-pushed the user-settings branch from eecac16 to 3f686d5 Compare November 7, 2017 18:47

vladimir-v-diaz reviewed Nov 7, 2017

View reviewed changes

Enhance settings section in README.md

44f95ad

lukpueh added 2 commits November 7, 2017 17:20

Misc user settings comments/log fixes

4ca9b04

Addressing @vladimir-v-diaz's review regarding comments and log statements related to user settings.

Move user settings patching after args parsing

dffa53a

Since parsing the user settings has verbose output, it is better to postpone it until after parsing command line tool arguments, based on which e.g. the verbosity level of the program is set.

Refactor ARTIFACT_EXCLUDES

3624c53

The setting lists patterns to exclude artifacts, hence the new better suited name: ARTIFACT_EXCLUDE_PATTERNS

vladimir-v-diaz reviewed Nov 7, 2017

View reviewed changes

lukpueh merged commit 40f1e1d into develop Nov 8, 2017

lukpueh deleted the user-settings branch November 8, 2017 17:50

		@@ -0,0 +1,3 @@
		from in_toto.user_settings import set_settings

		@@ -0,0 +1,3 @@
		from in_toto.user_settings import set_settings
		# Override settings in settings.py



		def get_rc():
		"""Load RCfiles from the usual places and return a dictionary of

		self.assertFalse("NOT_IN_TOTO_NOTHING" in env_dict)


		def test_set_settings(self):

Add user settings module #143

Add user settings module #143

Conversation

lukpueh commented Oct 26, 2017

coveralls commented Oct 26, 2017

vladimir-v-diaz 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

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

vladimir-v-diaz 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

vladimir-v-diaz Nov 1, 2017 • edited Loading

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

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

vladimir-v-diaz Nov 1, 2017 • edited Loading

Choose a reason for hiding this comment

coveralls commented Nov 1, 2017

coveralls commented Nov 2, 2017

lukpueh commented Nov 2, 2017 • edited Loading

coveralls commented Nov 6, 2017

lukpueh commented Nov 6, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 7, 2017

vladimir-v-diaz 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

lukpueh Nov 7, 2017 • edited Loading

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

coveralls commented Nov 7, 2017

coveralls commented Nov 7, 2017

coveralls commented Nov 7, 2017

Choose a reason for hiding this comment

vladimir-v-diaz commented Nov 7, 2017

vladimir-v-diaz Nov 1, 2017 •

edited

Loading

vladimir-v-diaz Nov 1, 2017 •

edited

Loading

lukpueh commented Nov 2, 2017 •

edited

Loading

lukpueh commented Nov 6, 2017 •

edited

Loading

lukpueh Nov 7, 2017 •

edited

Loading