Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
PG Lewis
@pglewis
one other big refactor I think should be high priority is moving all the field saving stuff out as well, even though it doesn't get us to nested fields any faster
it was on the metabox object, refactored to the field groups object, but doesn't really belong on either; persistence should be it's own area of responsibility
and metaboxes shouldn't have to know or worry about the details, so it can be customized
who knows, maybe someone wants to create some sort of multi-site field that can read/save from different blogs, refactoring the persistence they could just extend a base class and hand that to the metabox
Pods probably needs some of that for ACTs
Justin Sternberg
@jtsternberg
I like the general ideas, but am certainly leery of breaking back-compat. Also, though not elegantly OOP, overriding stored data setting/getting is currently possible through filters.
PG Lewis
@pglewis
does pass tests, and quick scan against the big example metabox from example-functions
data seems to load and save at a glance
PG Lewis
@pglewis
considering that was just a few hours of reworking/refactoring, I'm sure it could be cleaned up to be back compat if there is anything lingering
PG Lewis
@pglewis
Feel free to get me more scenarios to test it with or pound on it yourself. It's still in mostly proof of concept stage but should be 100% functional. Just needs more cleanup and further refactoring if it moves forward.
I live my life thinking about back-compat... every single time I touch Pods' code lol
doesn't mean I won't miss a thing or two, but it's at the forefront as I'm doing all this
PG Lewis
@pglewis
and my current philosophy working here is: if it doesn't pass tests locally, it doesn't get committed
re-working fields to support arbitrary nesting is going to be far hairier than jettisoning $meta_box was
"My brain took a beating to get it to work the way it currently works... Can't imagine the cerebral abuse required to get it to work recursively while not breaking back-compat. lol"
I understand you loud and clear now that I'm knee deep myself :)
Justin Sternberg
@jtsternberg
Ha ha ha ha! Yep, it's hairy! Hopefully I'll be able to dig in and help work on that soon. I'll be on a long flight tomorrow so will be happy to look over what you have so far
PG Lewis
@pglewis
@jtsternberg: some bolstered testing against CMB2::prop() in #447
PG Lewis
@pglewis
and the beginnings of some user mb specific tests in #448
PG Lewis
@pglewis
there will be a lot more where those came from
those were specifically added because they're things I had broken playing around but tests didn't detect
Justin Sternberg
@jtsternberg
@pglewis Seriously appreciative. The tests that ARE there were hard-earned, but I think we're still a ways off from full coverage.
PG Lewis
@pglewis
I fully empathize, Pods is the same way
Justin Sternberg
@jtsternberg
I'm sure! The long-game is that these tests improve tests for both libraries :)
PG Lewis
@pglewis
back-compat is a big constraint in refactoring and a necessary evil, so I'll bolster the tests as much as I can to catch those things
because we have to refactor, and we can't break back-compat lol
all my experimental branches currently pass all tests, including the new ones, and work for the example functions and the options page implementation in the snippets lib
which of course means it time for me to break them again
the snippets library is probably going to be fertile ground for more testing scenarios as well
Justin Sternberg
@jtsternberg
Yes, absolutely agree. So I take it your testing user profile fields as well
PG Lewis
@pglewis
yeah, user profile mb as in the example functions
it looks like the example functions registers an options page mb that's never hooked up, as far as I could find, so I replaced that one with the example code from the snippet lib
what I test against will grow as I go along too, I'll pull more from the snippet library, or any other source for testing stuff I can get my hands on
Justin Sternberg
@jtsternberg
You're right about options page snippet in example functions
PG Lewis
@pglewis
at least not an indicator that I'm going crazy yet
Justin Sternberg
@jtsternberg
Another good test bed would be the third party addons: https://github.com/WebDevStudios/CMB2#3rd-party-resources
yah, not yet. :P
PG Lewis
@pglewis
I'm always careful to be clear with my language, there is no definitive proof that I'm not going crazy, after all
and yeah, custom field types are high on my list of testing to expand to
currently back to evaluating if I can just remove the redundant field arrays stored
that branch is instantiating field objects when they're created now, so objects always exist
and the arrays keys are accessible via ->args, so we can still return that when requested
PG Lewis
@pglewis
but keeping both an array of arrays and an array of the objects seems redundant and full of confusion for me lol
pitfalls with back compat for sure, but I'm still slowly working around them
Justin Sternberg
@jtsternberg
agree, it's pretty terrible.. updating one of the arrays in the array doesn't update the corresponding object
PG Lewis
@pglewis
I /think/ it can be done
just harder than I originally thought lol
Justin Sternberg
@jtsternberg
worst case, can re-build the array on the fly if the fields array is requested, right?
PG Lewis
@pglewis
still, tests will improve regardless of whether or not this experiment proves worthwhile
and that's nearly as important as making this branch work