These are chat archives for WebDevStudios/CMB2

3rd
Sep 2015
Justin Sternberg
@jtsternberg
Sep 03 2015 01:12
@pglewis ping
PG Lewis
@pglewis
Sep 03 2015 01:12
acknowledged
Justin Sternberg
@jtsternberg
Sep 03 2015 01:12
re: https://github.com/WebDevStudios/CMB2/pull/449/files I think we should revamp to use the new fields/metabox API vs using the fields array
I can do that quick, but I wouldn't be able to verify that it works for the scenarios that you are testing against
PG Lewis
@pglewis
Sep 03 2015 01:13
what does it change?
PG Lewis
@pglewis
Sep 03 2015 01:31
not sure how/where it would be implemented or what it touches
Justin Sternberg
@jtsternberg
Sep 03 2015 01:31
no problem.. still thinking it through
PG Lewis
@pglewis
Sep 03 2015 01:32
my current branch passes all but 3 tests as it stands with the same tests that I have on our trunk fork (the same as what's in the PR)
it's pretty far diverged from trunk otherwise, though, as you might guess
but my focus has been on passing existing tests first to validate back-compat that far
then test more with real metaboxes, which would likely precipitate new tests to bolster things again
but if another refactor is beneficial, I'll not let that deter me
I haven't ripped into CMB2_Field too much yet
most of it has been on the CMB2 class, but that's all-encompassing
PG Lewis
@pglewis
Sep 03 2015 01:41
but definitely let me know what you're brainstorming, I can modify my course of action here to accommodate if needed
since you said "I can do that quick", I'd have to assume it's not a giant refactor. Either that or just a case of optimism ;)
Justin Sternberg
@jtsternberg
Sep 03 2015 02:01
Nevermind... the modifications to the tests are good
PG Lewis
@pglewis
Sep 03 2015 02:02
I needed to give explicit field names to the data arrays, that's the one oddball thing
but it makes sense that it should be that way... creation of the field is going to name them by field ID and not the original indexes 0, 1, 2 when omitted
God knows it'll find the one guy out there relying on those index values upon release though
Justin Sternberg
@jtsternberg
Sep 03 2015 02:04
No, that's the way it should be. I'm actually working on a method that will convert fields arrays to using the add_field methods
PG Lewis
@pglewis
Sep 03 2015 02:06
it'll still take field arrays without names, you just can't rely on those numeric indexes the way I've refactored... fields always get the name set on creation, and they're always created
but I wanna make sure I stay on the same page as you
if you're reworking add_field... well, obviously I'm hacking add_field too
in fact, all the nesting I built in the example was done exclusively via add_field
Justin Sternberg
@jtsternberg
Sep 03 2015 02:07
no, this is what I'm working on.. sounds like maybe a little overlap. Let me know: http://b.ustin.co/1bcue
PG Lewis
@pglewis
Sep 03 2015 02:07
since fields can be groups now, I can call add_field on a field group, not just a metabox
I didn't use add_group_field once for the example in the screenshot
Justin Sternberg
@jtsternberg
Sep 03 2015 02:08
hmmm.. let me try to re-do that diff
PG Lewis
@pglewis
Sep 03 2015 02:10
this gives you an idea of how far removed my branch is:
$meta_box doesn't even exist
and everything to do with fields and field collections is moved out to the new class
when boiled down at that point, it's basically a bunch of getters and setters and nonce management
PG Lewis
@pglewis
Sep 03 2015 02:16
and show_form, of course
Justin Sternberg
@jtsternberg
Sep 03 2015 02:17
does that conflict with your stuff?
basically, if they add fields an an old-school array, it loops through them and adds them the right way
PG Lewis
@pglewis
Sep 03 2015 02:18
probably, but it's not exactly a giant changeset
I'm trying to merge back up from your trunk, to ours, and up to my branch
test changes I put in our trunk and submit the PR from there
so if you make changes I'll end up merging them in on my own, if that means refactoring it for working the "new way"
Justin Sternberg
@jtsternberg
Sep 03 2015 02:20
that's a lot of setters/getters. Is there any reason there should be a method for all of them, vs one setter/getter?
PG Lewis
@pglewis
Sep 03 2015 02:20
my add_fields isn't even in cmb2
because it's easier to manage in the long run, IME
most of the tedium is in creating them, and phpStorm will generate skeletons for every class property I highlight
Justin Sternberg
@jtsternberg
Sep 03 2015 02:21
how/why? (not being argumentative, just want to understand)
PG Lewis
@pglewis
Sep 03 2015 02:22
if you have all your logic for all your classes properties in one method, that method can grow pretty big
and 9 times out of ten it's just if/elseif or switch/case anyway
also benefits from doing it that way are autocompletion and phpDoc with a keystroke
having those allows you to change the whole underlying implementation later if you want to
the getters and setters are what are exposed and used on the consuming side, and hide the details so you're more free to change them in the future
though a single getter/setter does that too
Justin Sternberg
@jtsternberg
Sep 03 2015 02:25
yah, that's my main question. Why not one (or fewer)
PG Lewis
@pglewis
Sep 03 2015 02:25
let me turn it on it's head for a moment, what's the benefit of not having separate ones?
PG Lewis
@pglewis
Sep 03 2015 02:33
with individual getters and setters you can phpDoc the return values properly for each one
Justin Sternberg
@jtsternberg
Sep 03 2015 02:33
yah, that's true. Curious, how does this handle custom metabox properties?
PG Lewis
@pglewis
Sep 03 2015 02:33
this in turn means my IDE knows when I call get_field_objects() and foreach through it, each element is a Field object
and i get autocomplete on the field object as a freebie
Justin Sternberg
@jtsternberg
Sep 03 2015 02:34
because users can (and do) add arbitrary data to the cmb array and then pull it out elsewhere
Justin Sternberg
@jtsternberg
Sep 03 2015 02:35
since we're renaming things, I think we definitely should rename the cmb_id property/methods to just id
just kludged to add it to it's own free form array that I tack back on with array_merge()
just there for back compat or I wouldn't have thought of it
Justin Sternberg
@jtsternberg
Sep 03 2015 02:36
how about back-compat for the prop method?
PG Lewis
@pglewis
Sep 03 2015 02:36
passes all tests, including some I added specifically for prop
Justin Sternberg
@jtsternberg
Sep 03 2015 02:36
since that's how the arbitrary values are retrieved currently
ah ok, you still have the prop method. should have looked
PG Lewis
@pglewis
Sep 03 2015 02:36
reworked to use the "new way"
everything internally in cmb2 should be using the getters/setters now
and helper functions too
outside code doesn't yet, but the back compat bits should be catching nearly all that
Justin Sternberg
@jtsternberg
Sep 03 2015 02:37
yah, makes sense. so my biggest opposition to the individual setters/getters is the non-DRY redundancy throughout this file. Just eats at me. :) not necessarily a good enough argument to remove, but...
PG Lewis
@pglewis
Sep 03 2015 02:38
so, ultimately, it's "doable"
explicit getters and setters are the one time I come off my "less code is always better" stump
or one of the rare times
but in the end, it's usually not much less code overall and the IDE can generate the skeletons
Justin Sternberg
@jtsternberg
Sep 03 2015 02:39
at least do this?
        case 'id':
        case 'title':
        case 'type':
        case 'object_types':
        case 'context':
        case 'priority':
        case 'show_names':
        case 'show_on_cb':
        case 'show_on':
        case 'cmb_styles':
        case 'enqueue_js':
        case 'fields':
        case 'hookup':
        case 'save_fields':
        case 'closed':
        case 'new_user_section':
            return call_user_func( array( $this, 'get_'. $property ) );
:D
PG Lewis
@pglewis
Sep 03 2015 02:39
a single getter/setter approach still has to have all your logic
lol
the whole point is the freedom to refactor more in the future...
when they're all explicit like that, if we change a method name it's picked up through refactor tools
dynamic things like that are not
if it really saved time in code or execution, sure why not
Justin Sternberg
@jtsternberg
Sep 03 2015 02:41
Ok makes sense
PG Lewis
@pglewis
Sep 03 2015 02:42
when it's dynamic, my IDE is more likely to miss it, so I shy away from that a lot
but it's really a minor point, just letting you know why I didn't do the call_user_func approach myself
Justin Sternberg
@jtsternberg
Sep 03 2015 02:44
So the trade-off is that the code is more readable but less readable by humans
(ish)
ugh, didn't finish my thought
PG Lewis
@pglewis
Sep 03 2015 02:45
the getters and setters litter things up a bit, that's my least favorite part
Justin Sternberg
@jtsternberg
Sep 03 2015 02:45
So the trade-off is that the code is more readable to IDEs but less readable by humans
PG Lewis
@pglewis
Sep 03 2015 02:46
phpStorm has convinced me somewhat that if my IDE reads it better, so do I lol
Justin Sternberg
@jtsternberg
Sep 03 2015 02:46
yah, haven't dove into phpStorm yet
PG Lewis
@pglewis
Sep 03 2015 02:46
like I said, the getter/setter litter is probably the biggest downside I see
but without having to actually write them all, how much do they get in my way?
just doesn't appeal to the "less code, always less code" training in me
Justin Sternberg
@jtsternberg
Sep 03 2015 02:47
nor me :)
PG Lewis
@pglewis
Sep 03 2015 02:48
but I have found as a pattern, they tend to be pretty maintainable and they do free you up from letting people get to deep in your implementation
Justin Sternberg
@jtsternberg
Sep 03 2015 02:48
I'd also like to hear your reasoning for having them all be individual properties instead of using the array thing.
PG Lewis
@pglewis
Sep 03 2015 02:48
and that's what I'm trying to address for starters, because the back compat part is the hardest part
I have grown a hate for literal strings in my code
can't auto-complete
global search for 'type' is a nightmare
whereas a getter, get_type() on an object, one click and I'm in the method
Justin Sternberg
@jtsternberg
Sep 03 2015 02:49
Ok, so IDE chaining stuff
PG Lewis
@pglewis
Sep 03 2015 02:49
arrays get messy debugging too
I really couldn't have done the nested fields at this point trying to maintain field arrays and field object arrays at the same time
and I'm not sure what the separate meta_box array was bringing to the table
Justin Sternberg
@jtsternberg
Sep 03 2015 02:50
yah, def. was due for some refactoring, and I'm super glad to be rid of the field arrays/objects stuff
PG Lewis
@pglewis
Sep 03 2015 02:51
those properties are properties of the metabox object... that's what object props are for the way I see it
Justin Sternberg
@jtsternberg
Sep 03 2015 02:51
yah, that's a fair statement
PG Lewis
@pglewis
Sep 03 2015 02:51
simplifies the data structure a lot when debugging, that screen-shot from a few days ago shows that
cmb2-nested-data.png
all your metabox properties are right there, not nested in an extra layer inside the metabox array
Justin Sternberg
@jtsternberg
Sep 03 2015 02:52
yah
PG Lewis
@pglewis
Sep 03 2015 02:53
but I'm not a hard-ass on code style, I've learned over the years to work in a when in Rome approach
Justin Sternberg
@jtsternberg
Sep 03 2015 02:53
This has been my baby for a long time, so I'm sure you understand the questioning. :D
PG Lewis
@pglewis
Sep 03 2015 02:53
I'm totally open to that, I'm never sold on what I'm doing, myself
Justin Sternberg
@jtsternberg
Sep 03 2015 02:54
I just want to be convinced that if it's not how I would do it, that it's actually better overall
PG Lewis
@pglewis
Sep 03 2015 02:54
this is to say: you are a developer
Justin Sternberg
@jtsternberg
Sep 03 2015 02:54
Not because I think my way is the best, but you only know what you know :D
PG Lewis
@pglewis
Sep 03 2015 02:54
totally on the same page
I'm glad to have it poked at for weaknesses
we both have to live with any major restructuring if it makes it out into the wild
Justin Sternberg
@jtsternberg
Sep 03 2015 02:56
certainly. I'm super-glad you're on board, and really appreciate your (and others') work on it
PG Lewis
@pglewis
Sep 03 2015 02:57
one of the things about code approaches that aid the IDE, is those same patterns also aid new coders
string constants as array keys give you no insights as to what other keys may be available
but I can look at a group of getters and setters for a class and get a lay of the land for how the interface is supposed to work
things that rely on dynamic bits mean I have to understand run-time context to understand that line
explicit method calls I can see what's happening at a glance and know whether or not it pertains to what I'm looking for / doing
Justin Sternberg
@jtsternberg
Sep 03 2015 02:59
yah, for sure
PG Lewis
@pglewis
Sep 03 2015 03:00
I'm still not sold 100% on the exact inheritance we're doing
where field group is extended by the metabox and field objects
in one sense it's logical: both of those can be a collection of fields
Justin Sternberg
@jtsternberg
Sep 03 2015 03:01
can you push your changes so I can see how it's going?
PG Lewis
@pglewis
Sep 03 2015 03:01
and it's what accelerated us toward being able to do nesting with a proof of concept
everything is pretty much on our fork right now
Justin Sternberg
@jtsternberg
Sep 03 2015 03:02
on the trunk branch?
PG Lewis
@pglewis
Sep 03 2015 03:02
not sure I even have any local uncommited changes yet
no, named after nested fields
nested-fields-trunk
Justin Sternberg
@jtsternberg
Sep 03 2015 03:03
ah, there it s
what do you have in mind, if NOT doing it this way?
PG Lewis
@pglewis
Sep 03 2015 03:03
there's still a lot of duct tape just to get things to pass tests and things are in mid-refactor
all fields appear to be working except any group fields at the moment
still a handful of things to track down there
I'm not sure I have a Plan B at the moment...
at the point I had achieved at least display of nested fields and things were pretty much back-compat, I figured this is pitch-able as solution
Justin Sternberg
@jtsternberg
Sep 03 2015 03:05
absolutely
PG Lewis
@pglewis
Sep 03 2015 03:05
though I think another refactor pass on the fields object will simplify the remaining sticky bits
my main concern was "can this even be done and be remotely back compat"
I was amazed that it just might be
but user mbs, options page mb, mb on pages all seem to save an load, images... multi images
just no groups yet until I complete the unborking there
and fields data structure is becoming the major bottle neck to that
CMB2 and the fields group objects are slowly getting hammered into better shape
so if this moves forward, the plan would be: finish up passing tests, test against metabox example code, test saving from group and nested group fields
and then: break tests again by refactoring fields
get it stable in a wave of refactoring, start the next
if it doesn't move forward, I need to learn my alternative approach
Justin Sternberg
@jtsternberg
Sep 03 2015 03:09
'just no groups yet until I complete the unborking there'
isn't that what this is about?
nested groups basically?
PG Lewis
@pglewis
Sep 03 2015 03:09
I know a lot more about the structure now than when I went in
all group fields are currently broken for at least either saving or loading
since the structure under the hood changed and I haven't tracked down the last bits of broken there
like 3 tests failing, which are likely the culprits
but it's getting into the field objects now... field options staying in synch
less and less things to fix in CMB2 and the Field Groups class
and more and more things I'm tracing around the rabbit hole in fields
Justin Sternberg
@jtsternberg
Sep 03 2015 03:11
gotcha
PG Lewis
@pglewis
Sep 03 2015 03:12
rendering is close to right, some options not getting set to put the button text on the add/remove buttons
so the borked part of groups is either saving, loading, or both
haven't even checked the meta table 'cause i figured I'd get it to pass tests first then see where it lands
last couple were starting to make me lose hair though lol
PG Lewis
@pglewis
Sep 03 2015 03:34
and this kludgy bit of code should at least display mostly right on page edit:
again, note I'm pretty much just using add_field() for everything since field objects can now do that
so, from a consuming side, the API can be very easy to do nesting with, no special fiddling
and it simplifies adding group fields in general
sorry, originally said "post edit", that's set on pages
Justin Sternberg
@jtsternberg
Sep 03 2015 03:38
So add_group_field will just be a shim?
PG Lewis
@pglewis
Sep 03 2015 03:38
not really sure where it lands after the dust settles
Justin Sternberg
@jtsternberg
Sep 03 2015 03:38
Fair enough
PG Lewis
@pglewis
Sep 03 2015 03:39
it wasn't really until after I had gotten the nesting part to work that I even realized... hey, I don't need add_group_field at all
a couple new helper functions could probably make it smoother and aid the OOP frightened
PG Lewis
@pglewis
Sep 03 2015 04:16
if I do sell you on this restructure, or something like it, we might be able to maintain the divergent branches via TDD
if you write tests that fail for what you're doing, then write code that passes, then push the tests
I can write code on the divergent branch that passes tests, in case that's easier than merging
Justin Sternberg
@jtsternberg
Sep 03 2015 13:34
The way you're doing it, It's a lot easier to see how the tests are migrating
PG Lewis
@pglewis
Sep 03 2015 19:07
I see some updates to trunk
Justin Sternberg
@jtsternberg
Sep 03 2015 20:11
yessir, a few
PG Lewis
@pglewis
Sep 03 2015 20:12
I'd like to take a few minutes to talk about how we want to move forward before tomorrow afternoon, if possible
friend's bachelor party tomorrow evening so I'll likely be switching gears from code-focus to whiskey focus for a night
Justin Sternberg
@jtsternberg
Sep 03 2015 20:13
ok, let's talk
PG Lewis
@pglewis
Sep 03 2015 20:14
what are your thoughts on the current refactor? Is this something we could consider continuing that direction?
Justin Sternberg
@jtsternberg
Sep 03 2015 20:14
yah, so far so good, though we still have to get add_field_group working, right?
PG Lewis
@pglewis
Sep 03 2015 20:15
add_field_group?
Justin Sternberg
@jtsternberg
Sep 03 2015 20:16
or add_group_field.. whatever lol
PG Lewis
@pglewis
Sep 03 2015 20:16
add_group_field should be working except for saving and/or loading
the only thing broken with groups in general afaik
display should be working
all other tests, and the metabox examples in the example functions should /display/ right, or close to right
minus what I haven't quite tracked down yet with the field array -> field objects conversion
group saving may need some hammering with the new data structure yet, I just haven't gotten that far yet
I'm not personally using add_group_field in my nesting example, but that doesn't mean I pulled it out or that it doesn't work
just wasn't a necessary thing to do the nesting since add_field() now works splendidly with group fields as well as a metabox
PG Lewis
@pglewis
Sep 03 2015 20:23
but if you setup the example functions in the repo, you should see the group field on a page like always
via add_group_field
just won't load/save
and I haven't really put any focus into the load/save yet because I was still cleaning up tests
iow: that was expected
since the refactor made it this far and is as close as it is to passing tests, I'm confident the back-compat part is doable
Justin Sternberg
@jtsternberg
Sep 03 2015 20:24
Hmm I switched to your branch and my test group metabox disappeared entirely
PG Lewis
@pglewis
Sep 03 2015 20:25
are you using the example from the repo?
Justin Sternberg
@jtsternberg
Sep 03 2015 20:25
For the most part
PG Lewis
@pglewis
Sep 03 2015 20:25
if it's a different test metabox than what I was testing I'd have to look at it and see
Justin Sternberg
@jtsternberg
Sep 03 2015 20:25
I didn't dig in too deep, so I'll see if I did something weird
PG Lewis
@pglewis
Sep 03 2015 20:25
I'm looking on a non-front-page page
and seeing the group mb
can add a new entry, just won't save
(or may be saving and won't load)
Justin Sternberg
@jtsternberg
Sep 03 2015 20:27
so maybe it's because I have previously saved values?
PG Lewis
@pglewis
Sep 03 2015 20:27
front page also appears to load right
user mb, options
all show for me
if I ignore data, it works perfectly lol
Justin Sternberg
@jtsternberg
Sep 03 2015 20:27
ha ha nice
PG Lewis
@pglewis
Sep 03 2015 20:28
(actually probably a few glitches in the display due to the couple remaining broken tests)
Justin Sternberg
@jtsternberg
Sep 03 2015 20:28
the biggest frustration with groups is the name/id attributes
PG Lewis
@pglewis
Sep 03 2015 20:28
but I'd like to verify you're seeing that as well
Justin Sternberg
@jtsternberg
Sep 03 2015 20:28
But I know @sc0ttkclark has talked about doing something more javascripty so we too many inputs
PG Lewis
@pglewis
Sep 03 2015 20:28
I can send you my function.php if that helps
though if you're seeing something broken I haven't come across, I'd like to get a copy of that
Justin Sternberg
@jtsternberg
Sep 03 2015 20:29
let me look again
PG Lewis
@pglewis
Sep 03 2015 20:30
pretend I have a magic wand I can wave to fix loading and saving :)
Justin Sternberg
@jtsternberg
Sep 03 2015 20:30
I merged my trunk in.. maybe that has something to do with it? (I doubt it)
want me to push mine?
PG Lewis
@pglewis
Sep 03 2015 20:30
I don't think you can merge the trunk changes in directly
I would revert those and run off the branch as is
confuses the matter
I'll be working on merging those in myself today
I already have them in my trunk
we're testing the nested-fields-trunk branch btw
Justin Sternberg
@jtsternberg
Sep 03 2015 20:31
I can and did. :D there's not that much actually (and I just removed the stuff I added yesterday around auto-field creation)
only changes are JS
PG Lewis
@pglewis
Sep 03 2015 20:32
changes to add_field in CMB2, n'est pas?
that's not even in CMB2 in my branch
that's in CMB2_Field_Group now
if the merge put that in CMB2 it's going to override the one I'm using
Justin Sternberg
@jtsternberg
Sep 03 2015 20:33
yah, when I merged, I remove those changes completely
PG Lewis
@pglewis
Sep 03 2015 20:33
and breakage ensues
Justin Sternberg
@jtsternberg
Sep 03 2015 20:33
don't believe me? check out my branch compared to yours. https://github.com/WebDevStudios/CMB2/tree/feature/nested-fields-trunk
PG Lewis
@pglewis
Sep 03 2015 20:35
to cut down on variable here, though... since I'm seeing the group fields displaying and you're not
Justin Sternberg
@jtsternberg
Sep 03 2015 20:36
sure, will use your branch
PG Lewis
@pglewis
Sep 03 2015 20:36
I would like you to test the code that's in our branch just to make sure
btw, saving for fields that aren't in a group fields should work fine
like in the user mb or options mb from the snippet lib
that's my enhanced version of the example functions, plus options
PG Lewis
@pglewis
Sep 03 2015 20:48
the special "about page" metabox shows, pretty much everything appears to be functioning
there are other things I need to do once it's passing tests and save/load and remaining refactor details are worked out
since we're instantiating fields immediately now, I need to create a test metabox with 250 fields
I need to know any performance impact when abused
Justin Sternberg
@jtsternberg
Sep 03 2015 20:51
sorry, caught up in work stuff again
PG Lewis
@pglewis
Sep 03 2015 20:52
np, I type fast and can be a nasty chat-bomber :o
Justin Sternberg
@jtsternberg
Sep 03 2015 20:52
it's no problem, as long as you're cool w/ me floating in/out of here
PG Lewis
@pglewis
Sep 03 2015 20:53
I'm pretty much around until I crash tonight, you're getting concentrated-PG this week
it's important to me to evaluate the full viability of this because it needs to move forward in some fashion before Pods 3 can move forward
and also, if I talk you into this much, my next target is a Fields refactor I'm probably going to have a tough time getting through congress :)
I would like to rid it of the $args array like CMB2 ditched meta_box
Justin Sternberg
@jtsternberg
Sep 03 2015 20:56
oof
PG Lewis
@pglewis
Sep 03 2015 20:56
again, they're all field properties and the extra layer to get to them doesn't make much sense to me, but moreover, it's becoming the bottle-neck I'm hitting at the final bit of clean up vs tests
but one punch to the gut at a time, I'll let you see what you think of where we've landed thus far
Justin Sternberg
@jtsternberg
Sep 03 2015 20:56
ha ha fair enough
PG Lewis
@pglewis
Sep 03 2015 20:57
the bottom line is, despite the litter of the getters/setters, I think the API does everything it needs to that way while insulating the details completely now
assuming we deprecated prop() and __get and hopped in a time-machine to when those got removed completely, the CMB2 class is getting fairly tight in that regard, I think
most of the "looks a little messy" stuff left is for back compat I think
I came from a VB6 world in another life
so, when I look at a class like CMB2, I don't think "this class encapsulates code and logic for working with my data". I think "This class encapsulates my data".
thus a good class often ends up being mostly getters and setters in that pattern
PG Lewis
@pglewis
Sep 03 2015 21:02
all it's concerned about is managing the data and providing the public interface for the world to do so
in that regard, all the getters and setters aren't duplication like instincts suggest
each property is it's own entity and may need special handling
quite likely each will have different return types
just to kind of explain the philosophy behind the approach a little better
the code here has been very good, but the data structure is where the biggest trouble showed up for nested field support, so I go with what I know for managing data structure
PG Lewis
@pglewis
Sep 03 2015 21:07
the intent is that the getters and setters are the majority of your exposed API
PG Lewis
@pglewis
Sep 03 2015 21:13
and from an aesthetic standpoint, I think $cmb->set_custom_property( $key, $value ) reads better to someone unfamiliar with some code than $cmb->prop( $key, $value); which doesn't tell you necessarily that a custom prop is being added
the explicitness, while being cumbersome to initially create, creates a readability I've found helpful for maintenance
PG Lewis
@pglewis
Sep 03 2015 21:19
I need no run-time context to know what that setter is doing