Dev meeting 2017 09 12

Agenda

• 3.4.0 beta (@GawainLynch)
  • milestone 3.4 / Blocking issues
• [RFC] Thoughts on deprecating all methods on the Content object #6985
• [RFC] Symfony 4/Flex & Silex 2 project filesystem layout #6982

e.g.

• Status on drop bear invasion (@YourGitHubID)

Actionable Items

Outcomes

Log

19:30] 
gawainlynch ping @bob @carson @lenvanessen @ross @sahassar


[19:30] 
ross pong


[19:30] 
bob pong


[19:30] 
carson o/


[19:31] 
gawainlynch OK, while we wait … @carson & @ross have you two had a look at #6982 ?


[19:31] 
boltissueball #6982 [open] [4.0] Phase II: Symfony 4/Flex & Silex 2 project filesystem layout https://github.com/bolt/bolt/pull/6982


[19:32] 
gawainlynch It is basically no code changes beyond paths, so really interested in your thoughts on layout


[19:32] 
ross I have, I like the layout


[19:33] 
gawainlynch Sweet … keen to hear what anyone else has to say, but I figured the only person out of the loop has been Carson in Mexico


[19:34] 
bob
Yeah, I like it too, but you knew that already


[19:34] 
ross does that match pretty much exactly what symfony is moving to?


[19:34] 
carson I don’t like templates/source - that should be in public


[19:34] 
templates are twig files, not js/css files


[19:34] 
gawainlynch ¿qué? source is source


[19:34] 
ross you don't want src files in public?


[19:35] 
carson maybe not, but they aren’t template files


[19:35] 
gawainlynch Yeah, my thought too … I get the Twig / JS distinction, but public isn't the place


[19:35] 
bob
I think it makes sense, assuming the “assets” get copied on the fly to public.


[19:35] 
~Keeping twig out is good, though.~ (edited)


[19:36] 
carson I also think it’s weird database and translations go under app, but config and templates do not. Seems inconsistent


[19:36] 
gawainlynch @carson I'm just following upstream as much as can be …


[19:36] 
ross in the real world though none of those exist if you use the composer install, all the app/view and app/src files stay in vendor


[19:36] 
bob That’s in paralel with SF 4.


[19:37] 
Personally, I was fine with app/config/, but keeping it in sync with SF is good, too


[19:37] 
carson @ross Of course, but we want user installs to match core mostly. (edited)


[19:38] 
gawainlynch There's a PR for that too :slightly_smiling_face:


[19:38] 
carson That wasn’t my point


[19:38] 
gawainlynch I know, being cheeky


[19:39] 
lenvanessen Pong


[19:39] 
carson what about `static` or something else other than `templates` and `public` for frontend assets that need to be built


[19:40] 
gawainlynch That … has … potential :slightly_smiling_face:


[19:40] 
carson or maybe `assets`


[19:41] 
gawainlynch That also has potential … thoughts from others?


[19:41] 
gawainlynch is leaning towards `assets` at the mo' but hasn't really thought it though


[19:42] 
bob `assets` could be confusing wit `{{ asset() }}`, though


[19:44] 
gawainlynch @bob: Important point there though, `assets` would be where things used in `{{ asset() }}` would be built


[19:44] 
ross any idea what symfony is using?


[19:44] 
bob
@gawainlynch Yes, exactly why i’m mentioning it. :slightly_smiling_face:


[19:44] 
gawainlynch None currently, I didn't dig that far


[19:45] 
bob
oh, wait..


[19:45] 
:thinking_face:


[19:45] 
In that case, might be a good idea


[19:45] 
gawainlynch :slightly_smiling_face:


[19:46] 
Let me have a quick dig … while I do: [RFC] Symfony 4/Flex & Silex 2 project filesystem layout #6982


[19:46] 
boltissueball #6982 [open] [4.0] Phase II: Symfony 4/Flex & Silex 2 project filesystem layout https://github.com/bolt/bolt/pull/6982


[19:46] 
bob
That’s the same one. :slightly_smiling_face:


[19:47] 
gawainlynch copy/pasta :epic_fail:


[19:47] 
[RFC] Thoughts on deprecating all methods on the Content object #6985


[19:47] 
boltissueball #6985 [open] [RFC] Thoughts on deprecating all methods on the Content object https://github.com/bolt/bolt/issues/6985


[19:47] 
bob
Let’s not deprecate stuff until the replacements are in place..


[19:48] 
Other than that, I can live with it


[19:48] 
gawainlynch That is the point of that RFC, and my previous RFC PR


[19:48] 
ross yes, that's the plan... I'm gonna PR the other methods


[19:48] 
bob
Ok, all good then


[19:48] 
gawainlynch That was easier than last time then :slightly_smiling_face:


[19:49] 
bob
.. because we quarreled enough about it then. :wink:


[19:49] 
ross @gawainlynch is it feasible to pass in aliases to the Twig functions via the legacy service? or will that not work with the new runtime separations


[19:49] 
gawainlynch Pfft … you just like Ross' PRs better than mine :stuck_out_tongue: `</kidding>`


[19:50] 
ross so for the intervening period we can get `record.next` and `record|next` to point to the same method


[19:50] 
bob
It’s just an `[RFC]` :wink:


[19:51] 
carson `|next` is a pretty generic filter though and could mean many different things in different situations (edited)


[19:52] 
gawainlynch @ross: Tossing over your question … There is (if I am understanding it correctly) nothing stopping us putting stuff into services and them accessing those services from legacy & the runtimes


[19:53] 
ross I might put together a proof of concept for that then since that would reduce the BC breaks considerably



[19:54] 
carson What does next even mean? Current record id + 1?


[19:55] 
gawainlynch ¯\_(ツ)_/¯


[19:55] 
I've never used it, ever


[19:55] 
bob Ha


[19:55] 
It _can_ mean that.


[19:55] 
If you have a listing at say `/pages`,


[19:56] 
and it shows items 1, 2, 3, 6, 10.


[19:56] 
then next for 3 would be 6.


[19:56] 
ross it's in the Twig api


[19:56] 
bob it uses the sorting, set in contenttypes.yml


[19:57] 
ross if you have pages/1 it links to the next page by date desc


[19:57] 
bob ^ @ross _IF_ `date desc` is the used order.


[19:57] 
ross true, that's the default


[19:57] 
carson Ok so it's based on the content types sorting config


[19:57] 
bob
yes


[19:57] 
ross I don't think many people use it, but... if it's in the docs then we need to BC protect it


[19:58] 
bob
We use it in the majority of sites we make


[19:58] 
if it has something that resembles a blog, I like to put it in


[19:58] 
ross I guess it makes more sense in a Blog style layout


[19:58] 
^ snap


[19:58] 
bob
yes, that.


[19:58] 
hehe


[19:59] 
carson what about `next_record`?


[20:00] 
bob `{{ record|next_record }}` :-1:


[20:00] 
carson The problem is the filter is a gobal function, where the next method was scoped to the record object.


[20:01] 
bob I think that’s the price we’ll have to pay, if we move away from methods on the record object.


[20:03] 
gawainlynch 6982 updated and pushed… even the PR text :slightly_smiling_face:


[20:03] 
I'm a bit uneasy on that one TBH


[20:04] 
bob
Yeah, not thrilled either now i see it like that..


[20:04] 
Assets looks kinda “lost”, there.


[20:04] 
gawainlynch No, I meant the Twig one


[20:04] 
It only takes one PR from Fabien and we are in a world of pain


[20:04] 
carson `{% setcontent asdf = next record %}` next being a constant string and record being the record object (edited)


[20:07] 
bob I’m not sure I like that syntax.


[20:09] 
carson Well reality is a lot of logic needs to be ran to determine the next record, so hiding it in a twig filter may lead to “hidden” performance “issues”


[20:10] 
ross The goal is to put it in a filter so we don't have to inject every service into the content objects


[20:10] 
bob
That should not be a reason to introduce complex-looking syntax.


[20:10] 
ross We can streamline it more once we have an API defined


[20:11] 
carson Ross - I agree about not injecting services (edited)


[20:12] 
And since they aren’t being injected they need to be some kind of global service that we can query


[20:12] 
moving it to a twig filter that expects a record object just pushes the problem from one place to another


[20:13] 
`{{ query.next(record) }}`


[20:14] 
^ That’s a separate service that we are calling with our record object and it doesn’t pollute the global twig function/filter names. Thoughts?


[20:15] 
gawainlynch I like the cleanliness … I personally would go for it, but I can already hear the complaints from the … complainers?


[20:15] 
bob
I’m not a big fan, for one.


[20:16] 
gawainlynch is lost on descriptive words ¯\_(ツ)_/¯


[20:16] 
bob
uploaded this image: not a big fan.jpg



[20:16] 
bob (sorry)


[20:18] 
carson @ross this is your area, what do you think?


[20:18] 
bob As you all know, I personally like `{{ record.next() }}`, but I can follow along with the necessity to change it, but I don’t like this alternative much.


[20:19] 
gawainlynch Unpopular suggestion inbound … what about a _service_ called `record`


[20:19] 
carson Nope


[20:19] 
bob
:ear: : (edited)


[20:19] 
carson Services shouldn’t be named like entity names


[20:20] 
Needs to be more action based than noun based


[20:20] 
gawainlynch I get that … but we're coming from further and further behind here


[20:20] 
carson ?


[20:21] 
gawainlynch Well, we haven't heard from Ross, but this goes into FE advocacy territory, and you and I are not building a convincing argument to convince them … and we're not in the good book already with that crew


[20:23] 
bob
Thing is, we _can_ explain and document most things.. After a few bumps, nobody minds `{{ asset() }}` anymore.


[20:23] 
gawainlynch I mean I like the `query` idea, and am not ready to capitulate yet … but currently I am not bringing anything to the table to add value on getting our PoV across the line


[20:23] 
bob
And _maybe_ all it takes is a better name for it, so I’m not shooting it down at all.


[20:23] 
ross We can fine tune the names at PR stage


[20:24] 
I'll get the code together first and we can discuss


[20:25] 
carson Well filters vs what I suggested are coded very differently


[20:25] 
bob
Sure.. seeing it in action will make it much easier to come up with better names.


[20:25] 
carson It’s not just naming is what I’m saying


[20:25] 
gawainlynch
> Well filters vs what I suggested are coded very differently
^


[20:26] 
bob :thinking_face:


[20:26] 
gawainlynch @bob: So if it was `{{ i_cant_say_no.next(record) }}` ?


[20:27] 
ross Personally if we aren't planning anything else for a next filter then I don't see the harm.


[20:28] 
It's a small mental overhead to switch to the filter


[20:28] 
bob Well, `{{ whatever.next(record) }}` and `{{ whatever.link(record) }}` could be good, I think


[20:28] 
ross And still reads naturally to non programmers


[20:28] 
bob For a certain value of `whatever`


[20:29] 
perhaps `{{ linker.self(record) }}`, `{{ linker.next(record) }}` ? (edited)


[20:30] 
`{{ helper.excerpt(record) }}` ?


[20:30] 
carson I like `linker`. It is a do-er word.


[20:31] 
bob except `linker.excerpt` doesn’t really fit.


[20:31] 
but a verb might be good.


[20:32] 
carson Right excerpt shouldn’t go there


[20:32] 
excerpt should be handled in the same way record.image is done


[20:33] 
It’s basically magic for a certain field in the record object right? we have more than one of those, it would make sense to group them together


[20:33] 
gawainlynch image & excerpt?


[20:34] 
carson If those are the only two then yeah


[20:35] 
There’s lots of magic there that I don’t agree with - I would be fine just deprecating them


[20:35] 
bob I’m against deprecating before we’ve agreed on a replacement


[20:36] 
gawainlynch Excerpt is kinda ugly in terms of magic/overhead … idk that attached to the entity is a good idea at all


[20:36] 
carson I was saying deprecate to remove


[20:37] 
gawainlynch OK, can we push those two until next week then?


[20:37] 
I am 17 hours into my day personally, and I don't think we're getting anywhere with this yet


[20:37] 
carson Maybe linker.next/previous/self(?) can be worked on as a PR until then?


[20:38] 
gawainlynch :+1:


[20:38] 
bob
Ok, let’s think on it


[20:38] 
ok


[20:38] 
gawainlynch Well anything to raise then?


[20:38] 
bob Yes


[20:39] 
3.4 beta.


[20:39] 
#6970 is open


[20:39] 
boltissueball #6970 [open] [3.4] Don't show files & directories that start with a dot https://github.com/bolt/bolt/pull/6970


[20:39] 
bob I am OK with it, gawain wanted to run it by @ross and @carson


[20:40] 
Or, carson specifically


[20:40] 
carson Ohh there’s a lot there. I’ll  read through it and talk with gawain offline (edited)


[20:41] 
gawainlynch Yeah, I didn't raise it just now for that reason :slightly_smiling_face:


[20:41] 
ross Yes I haven't had chance to look through. Will this evening


[20:41] 
bob
ok. Gawain and I discussed approach a ton already, so i think it’s mostly about the actual implementation.


[20:41] 
Finally: #6994 popped up today.. It doesn’t sit well with me.


[20:41] 
boltissueball #6994 [open] [ BUG ] Saving empty page throws NotNullConstraintViolationException exception https://github.com/bolt/bolt/issues/6994


[20:42] 
bob Is it something that should be sorted before we do a 3.4 public beta?


[20:42] 
gawainlynch Yes


[20:42] 
… and 3.3.4


[20:42] 
bob
Since it’s in the DBAL stuff, I assume it’s in `3.3` branch as well.


[20:42] 
gawainlynch I'm on it, and it is almost done :wink:


[20:42] 
bob
that


[20:42] 
awesome, mate. (edited)


[20:43] 
Ok, those two were on my list to bring up..


[20:43] 
gawainlynch I'm a little uneasy with what needs to happen, but it is more that we don't have a choice, and "previous rants" :wink:


[20:44] 
@bob regards the `assets/` thing in master … its a 4 line change to move it, and we are months out of doing anything public … so lets :shipit: and fine tune that one when better ideas come up


[20:44] 
bob Sure, want me to merge it (and the linked ones) in?


[20:45] 
gawainlynch It yeah … the Travis fail is their new PHP 7.2-RC1 build … I can push a Travis change but it isn't related to the PR iteself


[20:45] 
Hold off on the other two for a second though


[20:45] 
lenvanessen Sorry for the awayness:) Was at a client:)


[20:45] 
bob Okido


[20:45] 
Welcome back, @lenvanessen  :wink:


[20:46] 
(as an aside: Apple has “PHP 6'ed” the Iphone: They are going to release the iphone 8 and iphone X at the same time)


[20:46] 
lenvanessen If you need anymore info on #6994 just shout:)


[20:46] 
boltissueball #6994 [open] [ BUG ] Saving empty page throws NotNullConstraintViolationException exception https://github.com/bolt/bolt/issues/6994


[20:46] 
gawainlynch Oh Len, thanks for your work today … closed the two blockers on 3.4-beta


[20:46] 
… and opened one more blocker :stuck_out_tongue:


[20:46] 
bob
haha


[20:46] 
gawainlynch Nah, it is in hand mate, fix almost done


[20:46] 
lenvanessen I’m using multiple names now to send in issues on git


[20:46] 
So you won’t know it’s me


[20:47] 
gawainlynch I'll still know


[20:47] 
lenvanessen RTowarek is my polish alter ego


[20:47] 
gawainlynch Would would like the honours? (edited)


[20:47] 
bob “L3nv4n#ssen”


[20:47] 
nobody will know it’s you!


[20:47] 
</meeting>