15:32:03 <acozine> #startmeeting Documentation Working Group
15:32:03 <zodbot> Meeting started Tue Feb 19 15:32:03 2019 UTC.
15:32:03 <zodbot> This meeting is logged and archived in a public location.
15:32:03 <zodbot> The chair is acozine. Information about MeetBot at http://wiki.debian.org/MeetBot.
15:32:03 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
15:32:03 <zodbot> The meeting name has been set to 'documentation_working_group'
15:32:15 <acozine> #chair gundalow samccann
15:32:15 <zodbot> Current chairs: acozine gundalow samccann
15:32:18 <acozine> who else is around?
15:33:03 <acozine> nobody here but us chickens?
15:33:30 <acozine> #topic Big PR Review this week
15:34:07 <acozine> background: this is the third month we've done a day-long community review session focusing on assessing open PRs
15:34:07 <gundalow> #info https://www.ansible.com/blog/ansible-community-update-february-2019 & https://github.com/ansible/community/issues/407
15:34:29 <acozine> this month's session will focus, in part, on documentation PRs
15:35:07 <gundalow> Aim here is to help the various PRs that need both technical and Docs input to move
15:35:51 <acozine> as of right now we have 109 open PRs with the label `docs`: https://github.com/ansible/ansible/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+label%3Adocs
15:36:46 <acozine> I would like the community to focus on the oldest ones
15:36:50 <acozine> here's my reasoning:
15:37:34 <acozine> I'd like to hear whether the changes suggested in some of those old `docs` PRs are still of interest to the community, especially the ones that include code as well as documentation
15:38:21 <acozine> many of them need rebasing, but we don't want to rebase them if the changes are no longer needed or wanted
15:38:28 <samccann> +1 for cleaning up the old old stuff
15:38:44 <acozine> alongchamps: are you here for the DaWG meeting?
15:39:01 <alongchamps> I'm mostly watching, actually in a team meeting at the moment
15:39:04 <gundalow> acozine: That logic sounds good to me
15:39:19 <acozine> alongchamps: right-ho
15:39:36 <gundalow> so oldest first would give us https://github.com/ansible/ansible/pulls?q=is%3Apr+is%3Aopen+label%3Adocs+sort%3Acreated-asc
15:39:37 <acozine> I won't make you a chair for the moment, then
15:39:38 <gundalow> bah
15:39:42 <gundalow> https://github.com/ansible/ansible/pulls?q=is%3Apr+is%3Aopen+label%3Adocs+sort%3Acreated-asc
15:40:19 <acozine> yep, though for the review, we should ignore `WIP` ones
15:40:37 <acozine> https://github.com/ansible/ansible/pulls?utf8=%E2%9C%93&q=is%3Apr+is%3Aopen+label%3Adocs+-label%3Awip+sort%3Acreated-asc+
15:41:07 <gundalow> Good question, there re only 12 `WIP`, so I'd include them
15:41:14 <bcoca> you want to give WIP a quick look, in case people are going down a deaad end road, you want to stop them before they put in too much work
15:41:25 <acozine> #chair bcoca
15:41:25 <zodbot> Current chairs: acozine bcoca gundalow samccann
15:41:47 <acozine> bcoca: the ones I want to avoid are the ones that internal folks labelled WIP so I'd stop bugging them
15:42:05 <acozine> like 22959
15:42:39 <acozine> a few of them are in a limbo state - we've decided not to merge them, but we have not decided to close them
15:42:41 <bcoca> well, that is not really a docs pr, but a pr that includes docs for new features
15:43:06 <acozine> bcoca: true, and those are exactly the ones we'd like to target for the big community review
15:43:28 <acozine> the docs-only PRs are easy for the docs team to review
15:43:55 <acozine> the docs-and-code ones are harder, and we'd like to take advantage of the wider community attention to move them forward if we can
15:44:34 <acozine> that said, I guess it wouldn't hurt to get feedback on those about whether people think they are useful to pursue or not
15:44:41 <bcoca> i think we need a way to deal with 'crossover', its not only docs, but tests also
15:44:50 <acozine> gundalow: okay, I retract my "let's not look at WIP" comment
15:44:56 <bcoca> gettign approval from 'each section' docs/tests/code
15:44:56 <acozine> bcoca: yes!
15:45:30 <bcoca> i would hold of docs/tests till 'code' approval is there, since any changes in code can/probably will affect the other 2
15:46:08 <acozine> bcoca: yes, it's "code" approval/review we're mostly looking for from the Big PR Review day
15:46:20 <bcoca> we might need to discuss that in core meeting, to coordinate better, right now its 'chase X' cause I approved and PR is still in limbo
15:46:45 <gundalow> It's fine to open it up spend a minute ago them go `#action #1234 add to core agenda/Core Triage`
15:46:47 <bcoca> waiting_on_doc/test_approval?
15:46:50 <bcoca> labels
15:47:33 <bcoca> @gundalow i think its not worth for docs to review a 'mixed' pr when the code is still in flux
15:47:41 <acozine> bcoca: the point of the Big PR Review (which is happening on Thursday  - we are just doing some prep for it here) is to chip away at hte backlog
15:47:59 <gundalow> ie we will have mix of knowledge & skill set
15:48:05 <gundalow> as well as some of the Core Team available
15:48:20 <acozine> bcoca: most of these old PRs are not in flux, they are stuck/in limbo or have been forgotten
15:48:42 <bcoca> that makes more sense, i'm just trying to minimize frustration for docs reviewers, since we end up having 'docs reviewd' but 'code review pending' PRs stagnating
15:49:35 <bcoca> acozine: im not commenting on the full list, jsut a particular subset that we need to deal with (even outside of backlog)
15:50:08 <bcoca> that gets tricky and we dont have good workflow for, having 'all the minds' at same time available for review is rare
15:50:30 <acozine> bcoca: agreed that for incoming PRs a better distributed workflow would help
15:50:42 <acozine> when they need code/tests/docs all reviewed
15:51:04 <acozine> for this Thursday, though, getting some of the older PRs unstuck is my main goal
15:51:08 <gundalow> So we happy that we'll start with oldest PRs first?
15:51:15 <bcoca> didnt mean to drail, sorry
15:51:23 <gundalow> bcoca: it's all good
15:52:41 <acozine> bcoca: your `live` keyword PR is a case in point - it seems there's interest from the community, and if we could get it updated/rebased and reviewed on Thursday maybe it can finally get merged
15:52:52 <acozine> (that's 13620)
15:53:15 <acozine> I hate to lose good work
15:53:36 <acozine> just because we didn't have the bandwidth, or got distracted, or whatever
15:54:09 <sivel> 13620 hasn't been approved for the 2.8 release, so that is likely one reason it hasn't been worked on
15:54:13 <sivel> but is something we plan to work on
15:54:51 <gundalow> So `#action 13620 add to 2.9 potential list`, or roadmap_candidate
15:55:15 <acozine> sivel: so does that mean if we get community input and folks get it passing tests, we'd still hold it for the next release? or just that no core hours have been set aside to get it finished?
15:55:33 <sivel> acozine: we would probably hold it
15:55:38 <acozine> ah, good to know
15:56:30 <bcoca> acozine: live is not going to happen any time soon
15:57:30 <sivel> also, fwiw, we typically ignore PRs by the core team, as we assume that the author will drive it forward and ask the appropriate people for reviews
15:57:59 <sivel> at least in triaging
15:58:33 <acozine> gundalow: have you filtered out core PRs in the past Big PR Review days?
15:58:56 <gundalow> acozine: not directly, though I ignore `support:core`
15:59:47 <gundalow> hum, though I'm not sure if it should be `-label:support:core` or `label:support:community`
16:00:28 <acozine> do those two queries return different numbers of PRs?
16:00:38 <acozine> do we have PRs with both, or neither?
16:00:49 <samccann> we have prs with both... just checked
16:01:08 <bcoca> gundalow: do community or you would also need -label:surpport:netowrk
16:01:36 <gundalow> bcoca: yup, that's in the list, just didn't list it
16:01:48 <gundalow> TBH, support;network is down to 18 PRs, they've done great work
16:01:50 <bcoca> ^ we also need better workflow for 'multi support' PRs
16:02:06 <gundalow> bcoca: yup, totally agree
16:02:15 <bcoca> i.e community module that also updates module_utils/basic.py
16:02:26 <acozine> wow, yeah, 63 in the list for `label:support:community` and only 34 for `-label:support:core`
16:03:43 <gundalow> to take a few steps back. PR review day is about 1) Hitting the PR backlog with various sized hammers 2) engaging/motivating people that they can review and contribute.
16:03:43 <gundalow> So given, in someways I don't really care too much *what* queries we use. Though for Thursday we will be looking at `bug` PRs not covered by Core/Network/WG as they are more important that features
16:05:46 <acozine> here's what I'd like to avoid: someone spends an hour rebasing and reviewing a PR, then we don't merge it and it's still there in three months, still needs rebasing, and we're duplicating work
16:06:24 <gundalow> Yup
16:06:49 * gundalow is happy with acozine suggestion of oldest docs PR
16:06:58 <gundalow> acozine: what's next #topic?
16:07:10 <acozine> let me check the agenda
16:07:55 <acozine> #topic `loop` vs `with_*`
16:08:09 <acozine> related PRs: https://github.com/ansible/ansible/pulls?utf8=%E2%9C%93&q=is%3Aopen+is%3Apr+label%3Adocs+loop+in%3Atitle
16:08:58 <acozine> I think we can merge this one https://github.com/ansible/ansible/pull/47231/files
16:09:13 <acozine> it updates an example in the `crypttab` module
16:10:00 <acozine> has two shipits, though those folks must not be maintainers
16:12:14 <acozine> the example doesn't show the values of `ansible_mounts` but I imagine it's not a complex data set
16:12:45 <acozine> any thoughts, objections, concerns?
16:13:15 <acozine> okay, I'm going to merge
16:13:25 <samccann> :-)
16:13:40 <acozine> well, `rebuild_merge`
16:13:51 <samccann> does this change your mind - https://www.reddit.com/r/ansible/comments/98watc/creating_a_list_of_mount_point_names/
16:14:02 <samccann> sorry was trying to dig up what ansible_mounts might mean
16:15:59 <gundalow> (back in 5)
16:16:25 <acozine> samccann: thanks - one of the replies in that thread uses `loop` and the other `with_items` but there's only one lookup in each
16:16:36 <acozine> which I think means that the two would be equivalent
16:17:04 <samccann> ok cool
16:17:20 <acozine> I retract my statement about "it's not a complex data set"
16:17:54 <acozine> it's more accurate to say "no matter how complex the data, this loop is only looking at one thing at one level"
16:18:42 <samccann> heh
16:18:47 <acozine> the other PR updating an example needs rebasing, but also looks straightforward
16:18:49 <acozine> https://github.com/ansible/ansible/pull/47244/files
16:19:50 <samccann> do we ask the owner to rebase or do we take it over at this point?
16:20:12 <acozine> I'm happy to rebase this one
16:20:25 <acozine> I put my name on it weeks ago, and the rebase should be straightforward
16:20:29 <acozine> the change is just to one example
16:20:45 <samccann> kewl
16:21:10 <acozine> so that leaves the larger rewrite of loops:
16:21:27 <acozine> https://github.com/ansible/ansible/pull/47895/files
16:21:46 <acozine> I'd still like an example of a "here be dragons" change
16:21:48 <acozine> see https://github.com/ansible/ansible/pull/47895#discussion_r234054716
16:22:20 <acozine> can someone come up with an example `for_*` construction that would NOT provide the same results if changed to `loop`?
16:22:30 <gundalow> sivel: bcoca ^
16:23:26 <bcoca> subelements would be one i can think of
16:23:37 <sivel> there is a subelements filter
16:23:40 <sivel> that does the same thing
16:23:56 <bcoca> ah, that was 'on my list' ... didnt realize/forgot we added that
16:24:23 <acozine> subelements? so are these two not the same?
16:24:31 <sivel> I don't think we have any missing gaps.  The main point is UX (user experience)
16:24:40 <acozine> https://www.irccloud.com/pastebin/N5dGSYMr/with_*%20example
16:24:49 <sivel> and the UX of using a lookup in a loop, is considered "bad"
16:24:52 <acozine> and
16:24:53 <acozine> loop: "{{ sdb_info.partitions }}"
16:25:38 <sivel> acozine: we have `with_subelements` and then we also have a `|subelements` filter
16:25:55 <acozine> sivel: what does "using a lookup in a loop" look like in a task?
16:26:22 <sivel> `loop: "{{ lookup('items', sdb_info.partitions, wantlist=True) }}"`
16:26:35 <sivel> that is contrived, but an example
16:26:46 <acozine> thanks!
16:26:51 <sivel> that is equivalent of `with_items: "{{ sdb_info.partitions }}"`
16:27:01 <sivel> and also `loop: "{{ sdb_info.partitions }}"`
16:28:09 <acozine> okay, so here it's a matter of readability or style?
16:28:30 <sivel> So I wouldn't recommend to someone to switch from `with_fileglob: '*.txt'` to `loop: "{{ lookup('fileglob', '*.txt', wantlist=True) }}"`
16:28:39 <sivel> as the UX is bad with that change
16:28:51 <acozine> gotcha
16:29:05 <acozine> I'll see if i can work those examples in
16:29:38 <acozine> are there examples where switching to `loop` would actually change the output and/or break the playbook?
16:29:42 <acozine> I thought there were
16:29:58 <bcoca> mostly with_items: listiforgothas1sublist
16:30:28 <bcoca> https://gist.github.com/bcoca/030e1ad0b0a5cc741b5f246891d27954
16:30:42 <bcoca> ^ not that it shoudl 'not be converted' but that the conversion is tricky
16:30:45 <acozine> ahhhh
16:30:50 <acozine> okay, that makes sense
16:31:04 <bcoca> loop: [1, [2,3] ,4]|| flatten(1)
16:31:52 <bcoca> even trickier https://gist.github.com/bcoca/030e1ad0b0a5cc741b5f246891d27954
16:32:13 <acozine> so without `flatten(1)`, will `loop` return `1,4` for that? or `1,2,4`?
16:32:25 <sivel> yes, `with_items` has an implicit "flattening" that `loop` does not do.  `loop` is more closely related to `with_list`
16:32:41 <bcoca> but its also a flatten(1) , most 'conversions' i see use flatten() (unlimited)
16:32:48 <bcoca> loop == with_list
16:32:58 <bcoca> most people use with_items thinking its with_list
16:33:15 <bcoca> which causes corner case issues ... which is why loop was created to be 'unambigious'
16:33:56 <acozine> thanks sivel bcoca - I will update that PR today and ping you both back for a quick review - hope to merge it today/tomorrow
16:34:09 <acozine> oops, and we are over time for the DaWGs meeting today
16:34:13 <acozine> thanks everybody!
16:34:23 <samccann> \o/ loop progress!
16:34:38 <acozine> yeah, we actually addressed the agenda today!
16:35:02 <acozine> #endmeeting