| [07:55:28] | <mstenta[m]> | FYI I have a conflict during the first part of the dev call today. I'll try to join late if I can. |
| [11:59:51] | <symbioquine[m]> | farmOS dev call on now - all are welcome! https://meet.jit.si/farmos-dev |
| [12:31:51] | <symbioquine[m]> | ACTION uploaded an image: (280KiB) < https://matrix.org/oftc/media/v1/media/download/AWfsbo3Cu19g1VDXQsamOqwL... > |
| [14:21:22] | <mstenta[m]> | symbioquine: FYI I left a comment and minor nit-pick review on https://github.com/farmOS/farmOS/pull/1100 |
| [14:21:48] | <mstenta[m]> | Overall I think it looks good |
| [14:22:02] | <mstenta[m]> | The (string) casting would only be an issue if we ever wanted to add a non-scalar "supported property" in the future. |
| [14:22:49] | <mstenta[m]> | I can't really think of an example where that would be needed |
| [14:23:25] | <mstenta[m]> | And if there is, presumably we would add tests at the same time and realize that the string cast breaks it, and adjust accordingly. |
| [14:23:36] | <symbioquine[m]> | Sounds good |
| [14:23:49] | <mstenta[m]> | So I'm inclined to approve this, pending the very minor comment nit I requested :-) |
| [14:24:36] | <symbioquine[m]> | Seems like it should be merged then and if we decide to combine those code paths, that would be a separate/future change. |
| [14:25:37] | <symbioquine[m]> | Maybe worth adding tests to show how the cast behaves with complex/unexpected nested elements. |
| [14:26:06] | <mstenta[m]> | And regarding the "was this AI generated" question, I'm not worried about it, for the same reasons I gave earlier regarding Greg's PR: 1) the actual functional change is minimal (under the 15-line threshold that GCC uses, if we were to adopt that), and 2) the tests don't matter from a GPL perspective. |
| [14:27:17] | <mstenta[m]> | Let's add this as a topic for the upcoming monthly call, though... so we can nail down our own policy :-) |
| [14:27:20] | <symbioquine[m]> | symbioquine[m]: Ah, I see you already captured that in your feedback. |
| [14:28:35] | <mstenta[m]> | Oops... weird... OK... GitHub is acting weird |
| [14:29:01] | <mstenta[m]> | I accidentally commented twice... the second one was what I wrote first, and then discarded it... |
| [14:29:19] | <mstenta[m]> | Don't understand what happened there... |
| [14:29:58] | <mstenta[m]> | I actually removed the bit about your suggestion for adding a test in my final comment... |
| [14:30:40] | <mstenta[m]> | But I'll preserve it now in this second comment, and remove the duplicated sentences 😅 |
| [14:32:03] | <mstenta[m]> | Oh I see what happened... I had copied that first draft comment into the overall "Review" text box and forgot to remove it. |
| [14:32:23] | <mstenta[m]> | Ignore my ramblings... 😆 |