@diosmosis opened this Pull Request on October 13th 2021 Member

Description:

The adapter code is pretty similar for most directives, so I created a utility function automatically define the angularjs adapter based on some configuration (as also suggested by @peterhashair). It works and in the long run can keep the asset size from getting too much bigger, and will potentially also cut down on time to migrate directives. But it's rather complex (mostly for directives like piwik-dialog that don't use an isolate scope), so I'm not sure how good an idea it is. @tsteur @sgiehl (or anyone else) do you have any opinions?

image

Review

@tsteur commented on October 13th 2021 Member

Looks generally good @diosmosis . It only requires some angular test fixes as they are failing now.

@diosmosis commented on October 14th 2021 Member

@tsteur this PR is based off of https://github.com/matomo-org/matomo/pull/18127, just to double check that code was reviewed as well, correct?

@tsteur commented on October 14th 2021 Member

@diosmosis yes that included the other PR as I only looked and tested this PR but not the other one specifically (there was no needs-review label)

@diosmosis commented on October 15th 2021 Member

@tsteur :+1: thanks for the reply, I closed the other PR and will merge this when the build passes.

This Pull Request was closed on October 15th 2021
Powered by GitHub Issue Mirror