draft change: add_pooling_effect to more generic finder_fn

[pearsonca]

still need some other finder_fn examples to add to testing?

[codecov]

Codecov Report

Merging #222 (e543214) into develop (f0b58ee) will increase coverage by 0.00%.
The diff coverage is 100.00%.

❗ Current head e543214 differs from pull request most recent head 4cebf92. Consider uploading reports for the commit 4cebf92 to get more accurate results

@@           Coverage Diff            @@
##           develop     #222   +/-   ##
========================================
  Coverage    97.30%   97.30%           
========================================
  Files           14       14           
  Lines         1519     1524    +5     
========================================
+ Hits          1478     1483    +5     
  Misses          41       41           

Impacted Files	Coverage Δ
R/formula-tools.R	95.20% <100.00%> (+0.09%)	⬆️
R/model-design-tools.R	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[seabbs]

This all looks good to me and as suggested is certainly clearer. Aside from another test case, which we can sort of dream up I think, it is important we have a really clear news note here indicating that this is a breaking change. I actually wonder if we should have a news section for breaking changes specifically just to highlight this.

[pearsonca]

added another test to highlight ... capability + drafted NEWS update.

also noted during testing that enw_add_pooling_effect side effects it's argument. that's not generally the approach used elsewhere in the library, but the documentation does (sorta) frame it that way.

[pearsonca]

ugh, so this kinda goes beyond the limited scope of the issue, but:
should we address applying the interface ethos (this is public, therefore: should be documented as taking a data.frame, should not side effect argument, etc)?
OR should that be addressed as some comprehensive review targetting only that question => changes (+ writing developer guidance for that)? (i.e. something addressing #211)

[seabbs]

I just tweaked the proposed news item and incremented the development version. Just doing a final review and then will merge.

ugh, so this kinda goes beyond the limited scope of the issue, but:
should we address applying the interface ethos (this is public, therefore: should be documented as taking a data.frame, should not side effect argument, etc)?
OR should that be addressed as some comprehensive review targetting only that question => changes (+ writing developer guidance for that)? (i.e. something addressing #211)

I think this goes beyond the scope of this issue and we should deal with it in #211 as you suggest.

[seabbs]

LGTM