Actions Board View Rule #2249

michaelkleinhenz · 2018-08-20T11:19:18Z

This contains the state to metastate mapping action rule and the necessary changes to the existing templates.

A thorough and complete description can be found here: https://docs.google.com/presentation/d/18xKQmkf3UKlGoP7qLqUnrIYiflza674n_paUZXKuepA/edit

Please note that this PR contains 2 new source files (the actual rule and the test) and minor changes to 3 existing files. The rest of the files in the PR are changes to golden files.

codecov-io · 2018-08-20T18:43:53Z

Codecov Report

Merging #2249 into master will increase coverage by 1.73%.
The diff coverage is 66.53%.

@@            Coverage Diff             @@
##           master    #2249      +/-   ##
==========================================
+ Coverage   69.52%   71.25%   +1.73%     
==========================================
  Files         176      177       +1     
  Lines       16670    18079    +1409     
==========================================
+ Hits        11589    12882    +1293     
- Misses       3957     3994      +37     
- Partials     1124     1203      +79

Impacted Files	Coverage Δ
actions/rules/action_field_set.go	`59.18% <100%> (ø)`	⬆️
actions/rules/action_nil.go	`100% <100%> (ø)`	⬆️
actions/rules/action_state_to_metastate.go	`65.84% <65.84%> (ø)`
actions/actions.go	`61.11% <75%> (-12.23%)`	⬇️
controller/user.go	`34.04% <0%> (-4.99%)`	⬇️
migration/migration.go	`61.11% <0%> (-0.46%)`	⬇️
controller/space_iterations.go	`84.78% <0%> (+0.51%)`	⬆️
controller/area.go	`92.78% <0%> (+0.52%)`	⬆️
controller/iteration.go	`81.12% <0%> (+2.79%)`	⬆️
... and 8 more

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 a8fab53...e71b346. Read the comment docs.

kwk · 2018-08-21T09:55:46Z

actions/actions.go

@@ -72,14 +72,12 @@ func ExecuteActionsByChangeset(ctx context.Context, db application.DB, userID uu
 				Ctx:    ctx,
 				UserID: &userID,
 			}, actionConfig, newContext, contextChanges, &actionChanges)
-		/* commented out for now until this rule is added


Please line comments. Then it is easier to follow what is affected.

kwk · 2018-08-21T09:57:56Z

actions/rules/action_state_to_metastate.go

+)
+
+// ActionStateToMetaState implements the bidirectional mapping between state and column.
+type ActionStateToMetaState struct {


It is a little bit strange to have an action defined inside the rules package.

because the rule is the action. actions/actions would be weird, so does action/impls.

@michaelkleinhenz what do you mean with because the rule is the action?

The only reason there is a rules directory is to structure the files and to seperate the infra from the actual actions that may go away at some point as the actions will be stored somewhere else, like in the database. That rules directory is just to keep the files clean and structures. The rules are not rules but the semantic name is action.

kwk · 2018-08-21T10:01:01Z

actions/doc.go

@@ -3,7 +3,7 @@ Package actions system is a key component for process automation in WIT. It prov
 way of executing user-configurable, dynamic process steps depending on user
 settings, schema settings and events in the WIT.

-The idea here is to provide a simple, yet powerful "signal-slot" system that
+The idea here is to provide a simple, yet powerful "publish-subscribe" system that


You have almost the same comment without relating to a package or something in actions/actions.go. Please remove the latter one.

@michaelkleinhenz great. But please try to use one commit per comment so it is easy to follow as a reviewer.

kwk · 2018-08-21T10:10:43Z

actions/rules/action_state_to_metastate.go

+// make sure the rule is implementing the interface.
+var _ Action = ActionStateToMetaState{}
+
+func (act ActionStateToMetaState) containsUUID(s []uuid.UUID, e uuid.UUID) bool {


Instead of implementing this function here, I encourage you to use this:

fabric8-wit/id/slice.go

Line 110 in b0ee723

func (s Slice) Contains(v uuid.UUID) bool {

. I've implemented that function for you.

I get a []uuid.UUID from WorkItemTypeGroup.type. So I'd need to convert that into a id.Slice with an extra step, requiring more memory. So I'd suggest we update the model first to not use []uuid.UUID at all before this. I'd also say id.Slice should have something like fromSlice() or similar to make this more convenient.

No, you can just assign an []uuid.UUID to an id.Slice. See this example: https://play.golang.org/p/aIsXiZJZisU

package main import ( "fmt" ) type MySlice []int func main() { foo := []int{1,2,3} var mySlice MySlice = foo fmt.Println(mySlice) }

Fixed. 4764322

kwk · 2018-08-21T10:19:17Z

actions/rules/action_state_to_metastate.go

+	}
+	// removed in slice2
+	for _, elem := range old {
+		if !act.contains(new, elem) {


This causes another unnecessary l loop. For id.Slice we also have implemented a Diff method that you might want to copy the algorithm from and adapt it slightly: https://github.com/fabric8-services/fabric8-wit/blob/master/id/slice.go#L14 .

I need added and removed items seperately. So I don't think this improves on the complexity. Also, using id.Slice needs more memory here, for converting the Slices to id.Slice first and for the counter map.

kwk · 2018-08-21T11:56:10Z

actions/rules/action_state_to_metastate.go

+		if !ok {
+			return nil, errs.New("metastate value in value list is not of type string")
+		}
+		// this is important: we only add a new entry if there


This comment is very hard to follow. Please try to simplify it.

kwk · 2018-08-21T11:56:54Z

actions/rules/action_state_to_metastate.go

+	return nil, errs.New("given field on workitemtype " + wit.ID.String() + " is not an enum field: " + fieldName)
+}
+
+func (act ActionStateToMetaState) getStateToMetastateMap(workitemTypeID uuid.UUID) (map[string]string, error) {


Please provide a description with this function.

kwk · 2018-08-21T11:57:10Z

actions/rules/action_state_to_metastate.go

+	return stateToMetastateMap, nil
+}
+
+func (act ActionStateToMetaState) getMetastateToStateMap(workitemTypeID uuid.UUID) (map[string]string, error) {


Please provide a description with this function.

kwk · 2018-08-21T12:00:05Z

actions/rules/action_state_to_metastate.go

+	return &c1
+}
+
+// OnChange executes the action rule.


Please have a comment // OnChange implements action.Action.

No. It is not fixed.

I get the impression we're on different branches. This was fixed. I updated the comment to include the implements reference.

kwk · 2018-08-23T13:25:16Z

actions/rules/action_state_to_metastate.go

+		for _, board := range boards {
+			for _, column := range board.Columns {
+				if columnID == column.ID.String() {
+					thisColumn = column


can we break after this find or do we have to continue looping?

Breaking out of nested loops is not that easy and according to https://stackoverflow.com/questions/51996175/how-to-break-out-of-nested-loops-in-golang all solutions proposed there would make the code more complicated and hard to understand. Given that we're only dealing with a small number of columns here (I'd say in 99% of the cases, less than 5), I would say let's prevail code readability in favour of having the WIT count from 3 to 5.

Okay. I'm fine with this.

kwk · 2018-08-27T12:50:12Z

actions/rules/action_state_to_metastate.go

 	if err != nil {
-		return nil, nil, err
+		return nil, nil, errs.Wrap(err, "error loading space: "+err.Error())


No need to stringify the error here. Please get rid of err.Error(). You've already wrapped this error.

Fixed in feecf1c

The fix is fine. But maybe it makes sense to specify which space could not be loaded?

This is contained in the wrapped error. It contains the ID that is not found in the message. I don't see a reason for repeating this.

kwk · 2018-08-27T12:50:47Z

actions/rules/action_state_to_metastate.go

+	}
+	groups, err := act.Db.WorkItemTypeGroups().List(act.Ctx, space.SpaceTemplateID)
+	if err != nil {
+		return nil, nil, errs.Wrap(err, "error loading type groups: "+err.Error())


Same here, please get rid of err.Error().

Fixed in feecf1c

kwk · 2018-08-27T12:58:09Z

This contains the state to metastate mapping action rule and the necessary changes to the existing templates.

@michaelkleinhenz can you please explain in more details what this does and what it is good for? This description is only telling what is contained but not what it does.

kwk · 2018-08-27T13:10:46Z

actions/rules/action_state_to_metastate.go

+	return false
+}
+
+func (act ActionStateToMetaState) removeElement(s []interface{}, e interface{}) []interface{} {


I cannot find a test for this function. As simple as it looks this is really dangerous. First, it can be wrong. Second, I think you're leaking memory here. See this for an explanation:

NOTE If the type of the element is a pointer or a struct with pointer fields, which need to be garbage collected, the above implementations of Cut and Delete have a potential memory leak problem: some elements with values are still referenced by slice a and thus can not be collected.

(source https://github.com/golang/go/wiki/SliceTricks#delete-without-preserving-order)

Fixed in 35e6034

I've commented on the revision 35e6034. The issue is different now. The functions removeElement and contains should not be member functions.

Why? Sorry, but again, spilling over functionality that is only used to structure the code to package level does not makes things better but worse.

kwk · 2018-08-27T15:09:06Z

actions/rules/action_state_to_metastate.go

+}
+
+func (act ActionStateToMetaState) loadWorkItemByID(id uuid.UUID) (*workitem.WorkItem, error) {
+	wi, err := act.Db.WorkItems().LoadByID(act.Ctx, id)


No need to capture this function in a wrapper. The return value of both are the same.

? Both what?

The return values of the wrapper and the wrapped function.

Again, this is not for re-using functionality. This is to make the main method smaller and easier to understand what it does! Why are you trying to force your mind model into my code and internal wiring? Yes, sure, I could force everything into one gigantic method, like it is done in many other places in the WIT. Does this increase robustness? Does it increase readability? I don't think so. I could glue the onStateChange() and onColumnChange() back together and put it into onChange(). Sure, all of this is possible. But does it makes sense?

This is to make the main method smaller and easier to understand what it does!

I get your point you didn't get mine. Sure if I read loadWorkItem that is nice but this is an untested function and as you've mentioned in many other parts of the code we call act.Db.WorkItems().LoadByID(act.Ctx, id). That means, people know what this code does and it is not very secret or hard to read. So IMHO the mere fact that we're using it all over the code already outweighs the apparent readability which costs coverage and adds a layer over an already established function.

kwk · 2018-08-27T15:10:56Z

actions/rules/action_state_to_metastate.go

+	return metastateToStateMap, nil
+}
+
+func (act ActionStateToMetaState) addOrUpdateChange(changes change.Set, attributeName string, oldValue interface{}, newValue interface{}) change.Set {


Why is this a member function when act is not used anywhere inside?

Because there is no need to spill over purely internal functionality to package level. Encapsulation.

I didn't say you should put it on the package level but how about making it part of the change.Set?

Also, one could argue about the reason you gave for not putting it as a function on package level. Here's you may not be polluting the package level but you're requiring to create an object, namely ActionStateToMetaState, that is not needed for the execution of the function. I consider this worse because the prerequisites to run the function are just wrong.

I think you missed some of the basic ideas here. There is good reason for having Actions encapsulated as structs.

I think you missed some of the basic ideas here. There is good reason for having Actions encapsulated as structs.

You misunderstood me. I never said to not store actions as structs. All I asked for was to outsource helper functions as testable-building blocks. We've discussed that in today's Bluejeans meeting. As much as I don't like it but I can live with a test that does ActionStateToMetaState{}.addOrUpdateChange().

Ok. Fixed in 7dafce4.

kwk · 2018-08-27T15:11:28Z

actions/rules/action_state_to_metastate.go

+	return newChanges
+}
+
+func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) *change.Set {


Same here: Why is this a member function when act is not used anywhere inside?

Same reason as above.

kwk · 2018-08-27T15:13:02Z

actions/rules/action_state_to_metastate.go

+	return &c1
+}
+
+// OnChange executes the action rule. It implements rules.Action.


"executes the action rule" doesn't sound right and it is not helpful.

The same review comment was made in #2249 (comment) , which should've been fixed.

kwk · 2018-08-27T15:15:01Z

actions/rules/action_state_to_metastate.go

+	return newChanges
+}
+
+func (act ActionStateToMetaState) fuseChanges(c1 change.Set, c2 change.Set) *change.Set {


This function could need an explanation. And it I think it is best implemented as a member of change.Set, no? Then you can write:

var a, b change.Set c := a.fuse(b)

Also, why does it return a pointer to a change.Set? Below you explicitly dereference: return newContext, *actionChanges, nil.

I agree a.fuse(b) makes more sense to me.

No, because this implements a very specific functionality that is not generalizable. In this case, existing changes sould be updated rather than appended in order. This is very specific to this particular use case. I don't think this makes sense as part of change.Set. Also, I externalize to local functions for structuring the code and making is more understandable, not necessarily because there is a re-use case or it is generalizable functionality. It just does not make sense to write methods that have more than a few hundred lines. If it has, I segment them down into chunks that have a defined signature interface.

And it returns *actionChanges because of exactly this reason. This is a structural method. The caller needs *actionChanges.

That is somewhat confusing to me. The consumer of the fuseChanges function is func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error). Inside of that function you assign the actionChanges like so: actionChanges = act.fuseChanges(*actionChanges, executionChanges) and then you also return the changes like so: return newContext, *actionChanges, nil.

In the return you dereference the actionChanges, so for that part you don't need fuseChanges to return a pointer.

Since you also set a pointer that was a parameter before and also return the same value in OnChange, I doubt that this is thought through. One should be enough, either set (using the parameter) or return a new value but not do both.

I understand the first point. I don't understand the second point. Anyways, I changed the return type fo fuseChanges. 5001aa2.

jarifibrahim · 2018-08-27T18:18:37Z

actions/rules/action_state_to_metastate.go

+		}
+	}
+	// and return to sender.
+	return resultingWorkItem, changes, nil


We would be returning an empty workitem in resultingWorkitem if there were no changes. Is this expected?

Good find. Actually no, because the issue is somewhere else, but connected to this. In the case of the state change, isDirty is always true at l465. So this works, but for the wrong reasons. I'll update the method.

973fe2d. Checked again, indeed the isDirty was redundant. I think this happened because I copied and refactored the onColumnChange() method there. Again, good find.

jarifibrahim · 2018-08-27T18:26:30Z

actions/rules/action_state_to_metastate.go

+}
+
+// onStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns.
+func (act ActionStateToMetaState) onStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) {


Can you please add a comment stating what this function is supposed to return?
Depending on https://github.com/fabric8-services/fabric8-wit/pull/2249/files#diff-d2486097896184ec82948b273169e638R458 , the first value returned is either the new workitem or an empty workitem.

See above. It returns the updated WI. Added a comment.

jarifibrahim · 2018-08-27T18:43:53Z

actions/rules/action_state_to_metastate.go

+}
+
+// onBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state.
+func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) {


Can you please update the comment to include what this function is supposed to return?
This function returns either a workitem object with field values or an empty workitem object based on the value of wiDirty flag.

See above. Always returns the updated WI. 973fe2d.

michaelkleinhenz · 2018-08-28T08:21:36Z

[test]

michaelkleinhenz · 2018-08-28T09:38:32Z

rsync: command not found on the build.
[test]

jarifibrahim

We spoke about this on the PR call today. Please use workitem.ChangeSet method instead of creating Change.Set{} struct.

actions/rules/action_state_to_metastate_test.go

jarifibrahim · 2018-08-31T11:43:24Z

actions/rules/action_state_to_metastate.go

+		}
+	}
+	// return to sender
+	return resultingWorkItem, changes, nil


@michaelkleinhenz We're returning an empty workitem here if wiDirty is false. This isn't expected, right?
Can you please fix it?

Yeah. Fixed. 6f4d7fb.

Removed the redundant var in 3cb93a2.

jarifibrahim · 2018-08-31T12:21:36Z

actions/rules/action_state_to_metastate.go

+}
+
+// OnChange executes the action rule. It implements rules.Action. Returns the Work Item with eventual changes applied.
+func (act ActionStateToMetaState) OnChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) {


I do not like the fact that actionChanges is a pointer. It is passed to onStateChange, onBoardColumnChange by reference. If one of onStateChange/onBoardColumnChange fails, there is a possibility that the original object would be modified even when there was an error.

@michaelkleinhenz Can we get rid of pointers from OnChange, onBoardColumnChange and onStateChange methods? It is possible that original object is not modified when the error occurs in any of the methods but it would be better if we didn't use pointers where they are not necessary. This would help in debugging the failures as well.

Ok, that is actually a good reason. I don't think that can happen with the state rule, but in case of an error, the actionChanges could be left in an undefined state. I updated the signature. f83ff16. Note that this also required some changes to the other existing rules.

@michaelkleinhenz You're awesome. Thank you for changing it 🎉

jarifibrahim · 2018-08-31T12:26:59Z

actions/rules/action_state_to_metastate.go

+	return workitem, nil
+}
+
+func (act ActionStateToMetaState) getValueListFromEnumField(wit *workitem.WorkItemType, fieldName string) ([]interface{}, error) {


Why is wit a pointer? Let's not use pointers at unnecessary places.

Fixed. 5cd9aef.

jarifibrahim · 2018-08-31T12:28:40Z

actions/rules/action_state_to_metastate.go

+}
+
+// onBoardColumnsChange is executed when the columns change. It eventually updates the metastate and the state. Returns the Work Item with eventual changes applied.
+func (act ActionStateToMetaState) onBoardColumnsChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) {


Can we please not use a actionChanges as a pointer?

Done. See #2249 (comment)

jarifibrahim · 2018-08-31T12:28:51Z

actions/rules/action_state_to_metastate.go

+}
+
+// onStateChange is executed when the state changes. It eventually updates the metastate and the boardcolumns. Returns the Work Item with eventual changes applied.
+func (act ActionStateToMetaState) onStateChange(newContext change.Detector, contextChanges change.Set, configuration string, actionChanges *change.Set) (change.Detector, change.Set, error) {


Can we please not use a actionChanges as a pointer?

Done. See #2249 (comment).

jarifibrahim · 2018-08-31T17:09:13Z

actions/rules/action_state_to_metastate.go

+	}
+	boards, err := act.Db.Boards().List(act.Ctx, space.SpaceTemplateID)
+	if err != nil {
+		return nil, errs.Wrap(err, "error loading work item type: "+err.Error())


@michaelkleinhenz You can get rid of err.Error(). We're already returning it via errs.Wrap(err, ...) .

jarifibrahim · 2018-08-31T17:16:04Z

actions/rules/action_state_to_metastate.go

+	if act.UserID == nil {
+		return nil, nil, errs.New("userID is nil")
+	}
+	if actionChanges == nil {


@kwk Is this the correct way to check for empty actionChanges? Earlier it was pointer and this check made sense but now it represents []changes and each change is

type Set []Change type Change struct { AttributeName string NewValue interface{} OldValue interface{} }

Should this line be replaced with len(actionChanges) > 0 ?

The whole check does not make sense for the non-pointer scenario. This was in there to check that there is actually a usable actionChanges given. As we now always have an instance, this check is redundant. The actionChanges is allowed to be empty here.

jarifibrahim

Thank you for addressing all my concerns. Please wait for @kwk to also approve the PR.

michaelkleinhenz · 2018-09-07T09:55:18Z

[test]

michaelkleinhenz added 13 commits August 17, 2018 11:44

feat(actions): Actions system infra.

353edeb

fix(tests): Fixed the tests.

6ea5c68

fix(doc): Fixed documentation.

be9dae7

Merge remote-tracking branch 'upstream/master' into actions_infra

529be05

Merge branch 'master' into actions_infra

008f7ff

Merge branch 'master' into actions_infra

72ad9ce

fix(doc): Added doc.

ab89650

fix(code): Moved Change.

b7fa5c0

fix(code): Refactoring Change.Set.

5589d68

fix(formatting): Format fixed.

f625367

fix(staterule): Added state rule.

00305cf

fix(actions): Enabled state rule.

88fdfe9

Merge remote-tracking branch 'upstream/master' into actions_staterule

a60efd3

michaelkleinhenz changed the title ~~Actions Board View Rule and Controller Integration~~ Actions Board View Rule Aug 20, 2018

michaelkleinhenz requested a review from jarifibrahim August 20, 2018 11:23

michaelkleinhenz added 5 commits August 20, 2018 13:24

Fixed formatting.

532ca2f

Fix documentation.

fa75f51

Merge branch 'master' into actions_staterule

13d3f53

Fixed WIT.

9aab756

Fixed golden files.

94ca51e

kwk reviewed Aug 23, 2018

View reviewed changes

michaelkleinhenz added 5 commits August 24, 2018 13:35

Merge remote-tracking branch 'upstream/master' into actions_staterule

0edeacc

Fixed PR comments.

e832f8b

Merge branch 'master' into actions_staterule

13640c8

Merge remote-tracking branch 'upstream/master' into actions_staterule

1ce8bd8

Fixed group loading.

5b4d19c

kwk suggested changes Aug 27, 2018

View reviewed changes

kwk reviewed Aug 27, 2018

View reviewed changes

michaelkleinhenz added 7 commits August 27, 2018 16:04

Added test for contains. Fixes memory leak.

4b6acc6

Added test for removeElement.

35e6034

Added removeElement() test.

d867145

Merge remote-tracking branch 'upstream/master' into actions_staterule

dab1b62

Fixed slice creation.

f758549

Removed debug code.

649ef2a

Fixed equals.

58f5187

kwk reviewed Aug 27, 2018

View reviewed changes

jarifibrahim reviewed Aug 27, 2018

View reviewed changes

Fixed isDirty structural inconsistency in onStateChange().

973fe2d

michaelkleinhenz added 3 commits August 28, 2018 12:15

Fixed err check.

69d1af4

Fixed leftover bug.

9a8d624

Changed return type on fuseChanges.

5001aa2

jarifibrahim reviewed Aug 28, 2018

View reviewed changes

michaelkleinhenz added 2 commits August 28, 2018 15:45

Fixed tests, removed action instances.

7dafce4

Merge branch 'master' into actions_staterule

595316f

jarifibrahim reviewed Aug 31, 2018

View reviewed changes

Michael Kleinhenz added 2 commits August 31, 2018 13:47

Fixed empty return bug.

6f4d7fb

Removed redundant code.

3cb93a2

jarifibrahim reviewed Aug 31, 2018

View reviewed changes

Michael Kleinhenz added 2 commits August 31, 2018 17:42

Updated OnChange() signature to not work with cbr.

f83ff16

Removed pointer args.

9f4de7d

jarifibrahim reviewed Aug 31, 2018

View reviewed changes

jarifibrahim approved these changes Sep 4, 2018

View reviewed changes

michaelkleinhenz added 2 commits September 6, 2018 14:27

Merge remote-tracking branch 'upstream/master' into actions_staterule

5ff5380

Fixed some minor issues.

89c8cb4

Merge remote-tracking branch 'upstream/master' into actions_staterule

e71b346

Actions Board View Rule #2249

Are you sure you want to change the base?

Actions Board View Rule #2249

Conversation

michaelkleinhenz commented Aug 20, 2018 • edited Loading

codecov-io commented Aug 20, 2018 • edited Loading

Codecov Report

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

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

kwk Aug 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

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

michaelkleinhenz Aug 27, 2018 • edited by kwk Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkleinhenz Aug 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michaelkleinhenz Aug 27, 2018 • edited by kwk Loading

Choose a reason for hiding this comment

kwk commented Aug 27, 2018

Choose a reason for hiding this comment

michaelkleinhenz Aug 27, 2018 • edited by kwk 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

michaelkleinhenz Aug 28, 2018 • 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

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

michaelkleinhenz commented Aug 20, 2018 •

edited

Loading

codecov-io commented Aug 20, 2018 •

edited

Loading

kwk Aug 21, 2018 •

edited

Loading

michaelkleinhenz Aug 27, 2018 •

edited by kwk

Loading

michaelkleinhenz Aug 28, 2018 •

edited

Loading

michaelkleinhenz Aug 27, 2018 •

edited by kwk

Loading

michaelkleinhenz Aug 27, 2018 •

edited by kwk

Loading

michaelkleinhenz Aug 28, 2018 •

edited

Loading