Swappable models #389

m-vdb · 2018-01-10T10:06:48Z

Fixes #306

This PR aims at proposing a powerful replacement for base classes that were removed in #377.
⚠️ Do not merge for now, we need to test the version change on live projects

Changes summary:

split models between abstract models and actual models
define settings for swappable models + getters (à la get_user_model())
add Django migrations (❗️this point needs to be validated and tested with live projects)
add tests for the whole thing

jdufresne · 2018-01-10T13:25:30Z

Do you mind cleaning up the lint errors so the Travis tests will run?

Do we really need a to allow swapping all models? Or would handling just Event be sufficient? I'd prefer not to add additional complexity if it isn't handling a real use case.

m-vdb · 2018-01-10T15:37:28Z

Lint should be good after 581d0c9 and 3debd40. About swapping all the models: in my use cases, I have custom models for Event and Occurrence, but not for Calendar. I would argue that it is pretty useful to have this functionality for the occurrences as well. The swappability for the Calendar is not a must have in my opinion, but in some cases developers could have several calendar per user (for instance) and would like to enrich the calendar model. Since it was the same amount of work I decided to do it, but I can easily revert that.

I still need to figure out how to fix tests + migrations, but what do you think about this PR?

llazzaro · 2018-01-10T19:26:54Z

@m-vdb don't upgrade to 0.8.7 or do it in test server. we just removed this feature in 0.8.7

llazzaro · 2018-01-10T19:28:53Z

@jdufresne Let review this change, we could be breaking other people code.
Let me know what you think about this PR.
I will try to make some time at night to review this PR

jdufresne · 2018-01-11T02:41:16Z

I'll try to find some time to give this a deeper review.

keeth · 2018-01-12T01:21:34Z

I'm very interested in this as I use base classes heavily, and I find it hard to maintain.

jdufresne · 2018-01-12T06:09:34Z

@m-vdb Can you rebase this PR so Travis runs against the latest mater? I'd like to see CI green before the review continues.

m-vdb · 2018-01-12T07:45:43Z

my branch starts from develop, isn't it the way to go? Like I said in a previous comment, I still need to figure out how to make my test pass and test that in a live project. I am not sure about the way I handled the migrations.

jdufresne · 2018-01-12T12:47:53Z

Oh you're right. Sorry, I misspoke. I intended to ask to rebase on develop. When you have a chance, can you do that and squash the commits? It would help keep the change tidy and aid reviewing.

Nice, I think that the general approach looks good! I think the use of swappable will be easier to maintain than the previous base classes.

I'd like to see more tests and have them passing before this is merge ready. The tests go a long way towards allowing me to analyze the change. Let me know when that is ready and I'll continue the review. It might help to look at how Django tests the swappable User model as guidance.

Nice work!

jdufresne · 2018-01-12T12:50:43Z

tests/test_swapped_models.py

+        except ImportError:
+            raise RuntimeError(self.__doc__.strip())
+
+    @override_settings(SCHEDULER_CALENDAR_MODEL='schedule.Calendar', SCHEDULER_EVENT_MODEL='schedule.Event')


Why is this overridden to not be the alternate model? Shouldn't the new tests use the CustomEvent class?

I had an error with it but it might be related to my migration issue

ah I know why:

the tests are executed within the context of the tests project, using the tests.settings module

the migrations is meaningful only in the context of the tests.test_project, using the tests.test_project.settings

as a consequence, importing the migration file like I do in the tests results in an import error: the django.conf.settings.SCHEDULER_CALENDAR_MODEL isn't defined for tests.settings but it is defined for tests.test_project.settings

jdufresne · 2018-01-12T12:50:48Z

.travis.yml

@@ -27,6 +27,7 @@ before_script:
  - isort --check-only --diff
  - python -m django check --settings=tests.settings
  - python -m django makemigrations --settings=tests.settings --no-input --dry-run --check
+  - python -m django makemigrations test_project --settings=tests.test_project.settings


This feels a bit odd to me on the surface. Could this instead be a part of running the test? I'd like to avoid an extra test setup step if possible so that the tests are immediately runnable as before.

ok I'll see what I can do, I just wanted to avoid executing a script within the tests. but maybe I can just use call_command(), will try.

jdufresne · 2018-01-16T01:20:31Z

To resolve conflicts, do you mind rebasing instead of merging? It would make the commit history much cleaner and easier to review. Thanks.

m-vdb · 2018-01-16T07:01:59Z

done, sorry I missed that earlier. Here is where I am stuck: you can see that one of the tests that I introduced isn't passing. It is because I still haven't figured out how to migrate from the current version of django-scheduler to this one. Let me explain: if a user is under django-scheduler==0.8.8 and uses BASE_CLASSES and wants to switch to this new version, he will need to swap the models. And here comes the migration issue (cf. backtraces on travis).

Also, I realized something interesting. Once you define the swapped models, there are two migrations that are generated: one in the schedule module, and one in the test_project module.

I've tried to search Django documentation and other projects GH issues and I've found similar issues, but I still don't know exactly how to fix that:

villoid/django-oauth-toolkit@4ef9093 : warns about the migration when you swap a model. (when looking at their models, they are in the same situation as here: several swappable models with foreignkeys, I'll dig deeper)
Reusable apps can not be shipped with migrations openwisp/django-swappable-models#12 : looks like exactly the same issue

m-vdb · 2018-01-16T07:22:38Z

I've managed to make it somehow work, by following django-oauth-toolkit documentation. What users would have to do:

define a custom model without extending the abstract class
generate the migration
define the swapped settings and make the custom model inherit from the abstract class
generate a second migration: this one is a bit problematic because Django sees that you are adding new fields to the custom models, and some fields are defined non-nullable on the abstract classes, so it will prompt the user to provide one-off defaults. It's a small detail but in terms of UX it is a bit distressing.

Ideally, I think we would like to create the custom models as subclasses of the abstract models from the beginning, and generate a migration there, but this fails because of the following:

ERRORS:
schedule.Event.creator: (fields.E304) Reverse accessor for 'Event.creator' clashes with reverse accessor for 'CustomEvent.creator'.
	HINT: Add or change a related_name argument to the definition for 'Event.creator' or 'CustomEvent.creator'.
schedule.Event.creator: (fields.E305) Reverse query name for 'Event.creator' clashes with reverse query name for 'CustomEvent.creator'.
	HINT: Add or change a related_name argument to the definition for 'Event.creator' or 'CustomEvent.creator'.
test_project.CustomEvent.creator: (fields.E304) Reverse accessor for 'CustomEvent.creator' clashes with reverse accessor for 'Event.creator'.
	HINT: Add or change a related_name argument to the definition for 'CustomEvent.creator' or 'Event.creator'.
test_project.CustomEvent.creator: (fields.E305) Reverse query name for 'CustomEvent.creator' clashes with reverse query name for 'Event.creator'.
	HINT: Add or change a related_name argument to the definition for 'CustomEvent.creator' or 'Event.creator'.

It's easily changeable on the models but it might introduce backwards-incompatible changes? Also, I think that defining the related_name as "creator" is wrong, because related name defines the naming convention when we want to access to all events created by a user. By default it is User.events_set.all(), defining this as creator changes that to User.creator.all() which is just confusing. (reference). LMK what you think @jdufresne

coveralls · 2018-01-16T17:03:53Z

Coverage increased (+0.4%) to 80.473% when pulling 3c0ffa8 on m-vdb:mvdb/swappable-models into 51d36cb on llazzaro:develop.

coveralls · 2018-01-16T17:13:57Z

Coverage increased (+0.4%) to 80.473% when pulling 1391f25 on m-vdb:mvdb/swappable-models into 51d36cb on llazzaro:develop.

m-vdb · 2018-01-16T17:14:05Z

@jdufresne ok I think I'm good for all logical aspects, even for the migration. Tests are green now, can you take a look? Also, I think that this requires documentation and an extended migration guide to this new version; it shouldn't be too hard to write, I can do it before we merge this.

jdufresne · 2018-01-21T15:10:22Z

tests/test_swapped_models.py

+    def setUp(self):
+        super(TestSwappableModels, self).setUp()
+        # cleanup any possible leftovers before running
+        self._cleanup()


Why do you need to call _cleanup both before and after? Shouldn't one be sufficient?

if for some reason the test completely crashes, this makes sure the test "setup" isn't corrupted. I can easily remove, it is just a nice-to-have for the developers of this repo.

jdufresne · 2018-01-21T15:11:45Z

tests/settings.py

@@ -1,7 +1,7 @@
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 import os

-BASE_DIR = os.path.dirname(os.path.dirname(__file__))
+BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))


Was this change required to make the tests work? Can you explain?

see 8c45306 : __file__ is only absolute in python3, so it was never defined right in python2. This had no consequences before because this settings wasn't used throughout the tests but now it is.

jdufresne · 2018-01-21T15:13:00Z

.gitignore

@@ -9,3 +9,5 @@
 /dist/
 *.egg-info/
 /docs/_build/
+# generated in tests, shouldn't be versionned
+tests/test_project/migrations/0001_initial.py


I don't understand why this is in gitignore. Shouldn't this exist as a normal file?

the whole purpose of the test I wrote is to generate this file on the fly and verify that it is well formatted. If for some reason in the future someone inadvertently alters the shape of the generated file by changing the code of swappable models, then we will know it and the unit tests will break. Versioning this file doesn't make sense because it is generated by the tests. To avoid confusion for this repo's collaborators I chose to add this file to gitignore. If the test ever fails and the cleanup phase of the test fails to execute too, this file won't be visible with git status and a collaborator won't risk versioning it.

jdufresne · 2018-01-21T15:15:26Z

tests/test_project/settings.py

+
+BASE_DIR = os.path.dirname(os.path.dirname(__file__))
+
+SECRET_KEY = '_nc9_+ntllx!ptq5tzh96nn^$a#81#auhrdw#8t#+n0#4ig(lz'


Per 8343dde, please use an obviously fake secret for tests.

8fd293d ✅

jdufresne · 2018-01-21T15:17:43Z

tests/test_swapped_models.py

+        """
+        popen = subprocess.Popen(
+            [
+                'python',


This should probably use sys.executable

1391f25 ✅

jdufresne · 2018-01-21T15:19:07Z

tests/test_swapped_models.py

+                '--settings=tests.test_project.settings'
+            ],
+            stdout=subprocess.PIPE,
+            stderr=subprocess.PIPE,


As you're not using the input/output, you can use DEVNULL instead of PIPE.

in self._check_command_return() I do. In case the command exited with a non-zero return code, this method will display the stdout and stderr of the command, see:

https://github.com/m-vdb/django-scheduler/blob/8c45306aec5d2b8b1592f7c327cecfc6bb2558a7/tests/test_swapped_models.py#L52-L69

thongly · 2018-02-02T20:33:27Z

@jdufresne Is this still awaiting anything before a merge?

The base class is key, in my opinion, to address the problem of extensibility while avoiding the problems #377

thongly · 2018-02-03T03:40:05Z

UPDATE - This goes away with specifying the settings variables for the SCHEDULER_CALENDAR_CLASS. Perhaps a loud warning is enough to avoid this.

@m-vdb - I am testing a few things with this branch and tried inheriting from the various AbstractBaseClasses.

I noticed that I am getting a reverse accessor name clash. However, if the special related_name syntax is used, this goes away - as per this section in the Django docs

calendar = models.ForeignKey( SCHEDULER_CALENDAR_MODEL, on_delete=models.CASCADE, verbose_name=_("calendar"), related_name='%(class)s' )

thongly · 2018-02-03T04:46:48Z

Onto one of the final steps now, and I noticed that one of the migrations seems not to recognize the derived name of the Calendar class from settings.py.

Running migrations:
Applying schedule.0001_initial...Traceback (most recent call last):
File "manage.py", line 15, in
execute_from_command_line(sys.argv)
File "/usr/local/lib/python3.6/site-packages/django/core/management/init.py", line 371, in execute_from_command_line
utility.execute()
File "/usr/local/lib/python3.6/site-packages/django/core/management/init.py", line 365, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "/usr/local/lib/python3.6/site-packages/django/core/management/base.py", line 288, in run_from_argv
self.execute(*args, **cmd_options)
File "/usr/local/lib/python3.6/site-packages/django/core/management/base.py", line 335, in execute
output = self.handle(*args, **options)
File "/usr/local/lib/python3.6/site-packages/django/core/management/commands/migrate.py", line 200, in handle
fake_initial=fake_initial,
File "/usr/local/lib/python3.6/site-packages/django/db/migrations/executor.py", line 117, in migrate
state = self._migrate_all_forwards(state, plan, full_plan, fake=fake, fake_initial=fake_initial)
File "/usr/local/lib/python3.6/site-packages/django/db/migrations/executor.py", line 147, in _migrate_all_forwards
state = self.apply_migration(state, migration, fake=fake, fake_initial=fake_initial)
File "/usr/local/lib/python3.6/site-packages/django/db/migrations/executor.py", line 244, in apply_migration
state = migration.apply(state, schema_editor)
File "/usr/local/lib/python3.6/site-packages/django/db/migrations/migration.py", line 122, in apply
operation.database_forwards(self.app_label, schema_editor, old_state, project_state)
File "/usr/local/lib/python3.6/site-packages/django/db/migrations/operations/models.py", line 92, in database_forwards
schema_editor.create_model(model)
File "/usr/local/lib/python3.6/site-packages/django/db/backends/base/schema.py", line 249, in create_model
definition, extra_params = self.column_sql(model, field)
File "/usr/local/lib/python3.6/site-packages/django/db/backends/base/schema.py", line 141, in column_sql
db_params = field.db_parameters(connection=self.connection)
File "/usr/local/lib/python3.6/site-packages/django/db/models/fields/related.py", line 966, in db_parameters
return {"type": self.db_type(connection), "check": self.db_check(connection)}
File "/usr/local/lib/python3.6/site-packages/django/db/models/fields/related.py", line 963, in db_type
return self.target_field.rel_db_type(connection=connection)
File "/usr/local/lib/python3.6/site-packages/django/db/models/fields/related.py", line 877, in target_field
return self.foreign_related_fields[0]
File "/usr/local/lib/python3.6/site-packages/django/db/models/fields/related.py", line 634, in foreign_related_fields
return tuple(rhs_field for lhs_field, rhs_field in self.related_fields if rhs_field)
File "/usr/local/lib/python3.6/site-packages/django/db/models/fields/related.py", line 621, in related_fields
self._related_fields = self.resolve_related_fields()
File "/usr/local/lib/python3.6/site-packages/django/db/models/fields/related.py", line 606, in resolve_related_fields
raise ValueError('Related model %r cannot be resolved' % self.remote_field.model)
ValueError: Related model 'booking.Calendar' cannot be resolved

Upon inspection, this was due to the initial migrations for the booking app not having been created yet. The booking app is where the non-abstract Calendar lives.

However, trying to create the initial migrations for the app results in pointing back to the scheduler migrations.

File "/usr/local/lib/python3.6/site-packages/django/db/migrations/operations/fields.py", line 196, in state_forwards
state.models[app_label, self.model_name_lower].fields
KeyError: ('schedule', 'calendarrelation')

thongly · 2018-02-04T01:57:16Z

Perhaps splitting up the app might help alleviate the migration issues - as in this PR for vera.

I'm not familiar enough with the swappable migrations, but I'll try to fix based on this -
https://github.com/wq/django-swappable-models#migration-scripts

mikekeda · 2018-03-09T09:39:37Z

schedule/feeds/__init__.py

@@ -16,7 +16,7 @@ def feed_title(self, obj):
        return "Upcoming Events for %s" % obj.name

    def get_object(self, request, calendar_id):
-        return Calendar.objects.get(pk=calendar_id)
+        return get_calendar_model().objects.get(pk=calendar_id)


Instead of calling get_calendar_model() directly 2 times you can add Calendar = get_calendar_model().
General suggestion: I think the code will be cleaner if you use same approach for get_calendar_model() , get_event_model() and get_occurrence_model() for all your changes.

mikekeda · 2018-03-09T09:46:20Z

tests/test_utils.py

+
+    def test_get_calendar_model_swapped_bad_settings(self):
+        with override_local_settings(SCHEDULER_CALENDAR_MODEL='badsetting'):
+            with self.assertRaisesMessage(ImproperlyConfigured, "SCHEDULER_CALENDAR_MODEL must be of the form 'app_label.model_name'"):


Line too long https://www.python.org/dev/peps/pep-0008/#maximum-line-length
Same for lines: 169, 178, 183, 192, 197

mikekeda · 2018-03-09T09:50:15Z

tests/test_swapped_models.py

+    and then swap its models.
+    """
+    settings_path = os.path.join(settings.BASE_DIR, 'tests', 'test_project', 'settings.py')
+    updated_settings_path = os.path.join(settings.BASE_DIR, 'tests', 'test_project', 'updated_settings.py.tpl')


Line to long https://www.python.org/dev/peps/pep-0008/#maximum-line-length
Same for lines: 74, 104, 131, 143

mikekeda · 2018-03-09T09:51:07Z

schedule/feeds/__init__.py

@@ -16,7 +16,7 @@ def feed_title(self, obj):
        return "Upcoming Events for %s" % obj.name

    def get_object(self, request, calendar_id):
-        return Calendar.objects.get(pk=calendar_id)
+        return get_calendar_model().objects.get(pk=calendar_id)


Instead of calling get_calendar_model() directly 2 times you can add Calendar = get_calendar_model().
General suggestion: I think the code will be cleaner if you use same approach for get_calendar_model() , get_event_model() and get_occurrence_model() for all your changes.

mikekeda · 2018-03-09T09:55:38Z

schedule/utils.py

    occurrence_id = get_kwarg_or_param(request, kwargs, 'occurrence_id')
-    return Occurrence.objects.filter(pk=occurrence_id).first() if occurrence_id else None
+    return get_occurrence_model().objects.filter(pk=occurrence_id).first() if occurrence_id else None


Line too long https://www.python.org/dev/peps/pep-0008/#maximum-line-length
Same for lines: 117, 127, 224, 238, 243, 257

mikekeda · 2018-03-09T10:00:38Z

schedule/models/calendars.py

@@ -218,7 +228,7 @@ class CalendarRelation(models.Model):
    may not scale well.  If you use this, keep that in mind.
    '''

-    calendar = models.ForeignKey(Calendar, on_delete=models.CASCADE, verbose_name=_("calendar"))
+    calendar = models.ForeignKey(SCHEDULER_CALENDAR_MODEL, on_delete=models.CASCADE, verbose_name=_("calendar"))


Line too long https://www.python.org/dev/peps/pep-0008/#maximum-line-length

mikekeda · 2018-03-09T10:10:03Z

schedule/models/calendars.py

+
+    name: the name of the calendar
+    events: all the events contained within the calendar.
+    >>> calendar = Calendar(name = 'Test Calendar')


calendar = Calendar(name='Test Calendar')
Don't use spaces around the = sign when used to indicate a keyword argument or a default parameter value. https://www.python.org/dev/peps/pep-0008/#other-recommendations

mikekeda · 2018-03-09T10:34:07Z

schedule/models/calendars.py

@@ -104,46 +104,18 @@ def get_calendars_for_object(self, obj, distinction=''):


 @python_2_unicode_compatible
-class Calendar(models.Model):
+class AbstractCalendar(models.Model):
    '''


I know it's not your line, but could you please replace ''' with """ in docstring?
https://www.python.org/dev/peps/pep-0008/#documentation-strings

mikekeda · 2018-03-09T10:36:25Z

schedule/models/events.py

@@ -47,10 +47,10 @@ def get_for_object(self, content_object, distinction='', inherit=True):


 @python_2_unicode_compatible
-class Event(models.Model):
+class AbstractEvent(models.Model):
    '''


I know it's not your line, but could you please replace ''' with """ in docstring?
https://www.python.org/dev/peps/pep-0008/#documentation-strings

mikekeda · 2018-03-09T10:41:29Z

schedule/utils.py

    return event


 def get_calendar(event, request, **kwargs):
-    from schedule.models import Calendar
    calendar = None


It's not your line, but could you please remove it? I think it's pointless.

llazzaro · 2018-06-03T21:53:18Z

Thank you everyone for comments and reviews on this pull request.
I will soon try to review this PR.
@jdufresne are you ok with merging this changes? I didn't have time to review them, but if it's ok and you too... I will merge it soon. If not I will comeback here to add more comments

jdufresne · 2018-06-07T15:37:57Z

Sorry, @llazzaro. I won't have time in the near future to review this. If it looks good to you, please proceed.

lggwettmann · 2019-07-11T09:24:14Z

Is this going to merge any time soon? (Thanks for the great work, everyone!)

lggwettmann · 2020-04-03T16:57:31Z

What can I do to move this forward? I need to customize the occurrence model and would prefer to base that on the common project code. :) @llazzaro

pickyuptruck · 2020-04-29T12:40:44Z

any updates on this?

m-vdb · 2020-04-29T12:43:46Z

I am no longer using django-scheduler, so I think it might be best if someone took over :)

m-vdb mentioned this pull request Jan 10, 2018

review migrations in virtualenv #306

Closed

jdufresne self-requested a review January 10, 2018 13:35

jdufresne reviewed Jan 12, 2018

View reviewed changes

m-vdb force-pushed the mvdb/swappable-models branch 2 times, most recently from aa42c40 to 76aea1e Compare January 15, 2018 10:51

m-vdb added 15 commits January 16, 2018 07:46

Calendar model becomes swappable

94c11ce

Event model becomes swappable

2905fbb

Occurrence model becomes swappable

1ecd850

define settings for swappable models

f7ba9cf

define getters for all swappable models

64ee36b

use getters all over the place

b08d22f

use a test_project module to test the behaviour

8377a4f

add Django migrations

a35ed98

add tests for swapped models

1231c24

add instruction for generation migration in travis.yml

292e89e

remove unused import

fa50a87

fix import order after isort run

b5922d5

remove extra setup in Travis and use subprocess

4d4ba21

fix migration conflicts

dc68a53

add tests for model getters

98d64d9

fix migration for good (after merge)

4002ffe

m-vdb force-pushed the mvdb/swappable-models branch from c7d09d3 to 4002ffe Compare January 16, 2018 06:50

m-vdb added 3 commits January 16, 2018 08:28

modify previous migrations to point to the SWAPPABLE settings

afeff2b

remove Event.creator.related_name because it is confusing

02189d8

enhance TestSwappableModels and make the tests pass

3c0ffa8

__file__ is only absolute in python3

8c45306

jdufresne reviewed Jan 21, 2018

View reviewed changes

m-vdb added 2 commits January 21, 2018 19:38

Use an obviously fake SECRET_KEY for tests (bis)

8fd293d

use sys.executable in tests

1391f25

m-vdb force-pushed the mvdb/swappable-models branch from 9661a38 to 1391f25 Compare January 21, 2018 18:47

mikekeda reviewed Mar 9, 2018

View reviewed changes

llazzaro mentioned this pull request Jun 3, 2018

RE: Removed SCHEDULER_BASE_CLASSES #419

Open

m-vdb closed this Oct 22, 2021


		BASE_DIR = os.path.dirname(os.path.dirname(__file__))

		SECRET_KEY = '_nc9_+ntllx!ptq5tzh96nn^$a#81#auhrdw#8t#+n0#4ig(lz'

Swappable models #389

Swappable models #389

Conversation

m-vdb commented Jan 10, 2018

jdufresne commented Jan 10, 2018

m-vdb commented Jan 10, 2018

llazzaro commented Jan 10, 2018

llazzaro commented Jan 10, 2018

jdufresne commented Jan 11, 2018

keeth commented Jan 12, 2018

jdufresne commented Jan 12, 2018

m-vdb commented Jan 12, 2018

jdufresne commented Jan 12, 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

jdufresne commented Jan 16, 2018

m-vdb commented Jan 16, 2018

m-vdb commented Jan 16, 2018 • edited Loading

coveralls commented Jan 16, 2018 • edited Loading

coveralls commented Jan 16, 2018 • edited Loading

m-vdb commented Jan 16, 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

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

m-vdb Jan 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

thongly commented Feb 2, 2018

thongly commented Feb 3, 2018 • edited Loading

thongly commented Feb 3, 2018

thongly commented Feb 4, 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

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

llazzaro commented Jun 3, 2018

jdufresne commented Jun 7, 2018

lggwettmann commented Jul 11, 2019

lggwettmann commented Apr 3, 2020 • edited Loading

pickyuptruck commented Apr 29, 2020

m-vdb commented Apr 29, 2020

m-vdb commented Jan 16, 2018 •

edited

Loading

coveralls commented Jan 16, 2018 •

edited

Loading

coveralls commented Jan 16, 2018 •

edited

Loading

m-vdb Jan 21, 2018 •

edited

Loading

thongly commented Feb 3, 2018 •

edited

Loading

lggwettmann commented Apr 3, 2020 •

edited

Loading