laravel-model-settings icon indicating copy to clipboard operation
laravel-model-settings copied to clipboard

Consistent settings merging and performance improvements

Open lucianholt97 opened this issue 4 years ago • 4 comments

closes #70

lucianholt97 avatar Sep 27 '20 15:09 lucianholt97

@glorand I would appreciate your feedback and help to resolve conflicts with existing tests.

lucianholt97 avatar Sep 28 '20 13:09 lucianholt97

@lucianholt97 @colq2 thanks guys for the contribution. I agree with your observations, the only issue is that this PR is to heavy to be merge as one. Probably I will split in multiple items and will be delivered in the newx major version of the package. The package is used on many projects and want to avoid introduce bugs or "malfunctions".

glorand avatar Nov 11 '20 13:11 glorand

@glorand good to hear from you. Thanks for the feedback, I agree that this is more appropriate for a major release. @colq2 and I also wondered if we can implement some kind of inheritance between model settings. E.g. group settings are inherited as default settings by each model unless the model has its custom setting. We ran into n+1 query issues but this was before I introduced the cache-all strategy. What do you think?

lucianholt97 avatar Nov 11 '20 13:11 lucianholt97

Code Climate has analyzed commit 973ff1e0 and detected 54 issues on this pull request.

Here's the issue category breakdown:

Category Count
Style 39
Complexity 1
Clarity 14

View more on Code Climate.

codeclimate[bot] avatar Jan 14 '22 20:01 codeclimate[bot]

Code Climate has analyzed commit 61569b74 and detected 57 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 2
Style 40
Clarity 15

View more on Code Climate.

codeclimate[bot] avatar Sep 17 '22 16:09 codeclimate[bot]