14:01:09 <acozine> #startmeeting DaWGs Mini Docs PR Review Day
14:01:10 <zodbot> Meeting started Tue Apr 20 14:01:09 2021 UTC.
14:01:10 <zodbot> This meeting is logged and archived in a public location.
14:01:10 <zodbot> The chair is acozine. Information about MeetBot at http://wiki.debian.org/MeetBot.
14:01:10 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
14:01:10 <zodbot> The meeting name has been set to 'dawgs_mini_docs_pr_review_day'
14:01:27 <samccann> woot woot!
14:01:30 <acozine> #topic goals and ground rules
14:01:51 <acozine> #info our goal today is to review (and merge if possible) as many docs PRs as possible
14:02:23 <tadeboro> o/
14:02:33 <acozine> #info ground rules: all are welcome
14:02:42 <acozine> #chair samccann tadeboro
14:02:42 <zodbot> Current chairs: acozine samccann tadeboro
14:03:08 <acozine> #info if you want to be a "chair" of the meeting, just wave like this `o/` or say hi
14:03:20 <samccann> shall we mark each PR we review with the pr_day label so we can go back and track the results?
14:03:27 <acozine> samccann: yes, great idea
14:03:42 <samccann> ok I'll take that on as a task as we go along
14:04:58 <acozine> #info we follow the Ansible Code of Conduct: https://docs.ansible.com/ansible/latest/community/code_of_conduct.html
14:05:53 <acozine> okay, since andersson007_ suggested some PRs to look at right before the meeting started, let's begin with those
14:06:54 <acozine> #info PRs from the ansible/ansible repo are welcome, so are PRs from collections repos
14:07:01 <acozine> let's look at https://github.com/ansible/community-docs/pull/5
14:08:25 <samccann> do we info the PRs as we look?
14:08:30 <acozine> samccann: sure
14:08:39 <samccann> #info https://github.com/ansible/community-docs/pull/5
14:09:27 <acozine> heh, this PR contains words of wisdom:
14:09:52 <acozine> `Look through the currently open pull requests in the collection repository. You can help by reviewing them. Reviews help move pull requests to merge state. Some good pull requests cannot be merged only due to a lack of reviews. For more information how to provide a good review, refer to the review checklist. And it is always worth saying that good reviews are often more valuable than pull requests themselves.`
14:09:56 <samccann> so it may need a copy edit. Should we pick one of us to do that while others to a more substantive review/suggested edits so we aren't clobbering one another?
14:10:15 <acozine> sure
14:10:23 <acozine> who wants to do a copyedit?
14:10:26 <acozine> I see one typo
14:10:54 <samccann> if no one else hops in, I can do a copyedit while you and others read through more for comprehension, questions, etc
14:11:09 <samccann> tho now the pressure is on to make sure I find that typo!!
14:11:21 <acozine> heh
14:11:35 <lmodemal> late to the party
14:11:40 <acozine> #chair lmodemal
14:11:40 <zodbot> Current chairs: acozine lmodemal samccann tadeboro
14:11:44 <acozine> welcome!
14:12:10 <tadeboro> Hmm, I have some questions about the content itself. For example, do we really want to mash together code development and collection development in the quickstart?
14:12:49 <acozine> tadeboro: good question
14:13:23 <tadeboro> I will probably add a reviw to that PR and suggest that we mybe split it in two.
14:13:48 <tadeboro> Because that quickstart is anything but quick right now ;)
14:15:11 <acozine> heh
14:15:17 <acozine> I was wondering about the title
14:16:09 <acozine> I might call it "Making your first contribution" or something
14:17:44 <acozine> we use the words "developer" and "development" to mean so many different things, but this seems to be about getting new contributors up to speed
14:18:05 <acozine> tadeboro: you're right that for a "quickstart" it's pretty long
14:18:27 <tadeboro> And now that I read things through, the purpose is to help people start contributiing to collections. So the docs definitelly need to drop all ansible/ansible git repo stuff because it is irrelevant.
14:19:55 <abadger1999> Taking my kid to school today.  I may not get back in time for the meeting (if we're not skipping the meeting).
14:20:00 <samccann> I've just started reading the quickstart - should we change the example away from `community.mysql` to something like `community.example` so there's no risk of clashing with the actual community collection (assuming it either exists or someone may create it)?
14:20:38 <samccann> Thinking about someone who uses this to learn, forgot they created it, and then end up clashing with the real one a year later or something? Or am I being to docs paranoid ;-)
14:21:31 <samccann> oh ignore me
14:21:45 <samccann> I was reading it as creating a collection, not opening a pr.. doh!
14:22:24 <acozine> abadger1999: drive safe
14:22:45 <acozine> the meeting today will be the docs review day
14:22:54 <acozine> and it will go past the regular meeting time
14:23:01 <acozine> so feel free to join us when you get back
14:23:54 <samccann> ok another noob question - I don't ever use ssh to when I bring a repo down locally because I did that when I started on ansible/ansible and ended up merging a commit w/o a PR.
14:24:11 <acozine> woopsie
14:24:26 <samccann> Should we remove that from the quickstart and just leave the HTTPS option? or is it not possible for a noob like me to do something like that anymore in gitland?
14:25:09 <acozine> I'd leave just one option, and maybe add a link if there's a good page in the GitHub docs about cloning repos
14:25:19 <acozine> good catch
14:25:31 <tadeboro> The guide uses fork of the repo, so using SSH should be OK. Non-core people cannot break things.
14:26:01 <acozine> oops, I never changed the topic
14:26:04 <acozine> #topic https://github.com/ansible/community-docs/pull/5/files
14:26:25 <samccann> ok gonna leave that for y'all to comment on in the PR directly since there's a difference of opinion there ;-)
14:26:29 <acozine> tadeboro: I like your optimism and confidence
14:27:18 <tadeboro> IIRC, GitHub is phasing out the "push using username and password" functionality, so going HTTPS in instructions might not be the best here.
14:29:25 <acozine> tadeboro: ah, good to know
14:29:43 <acozine> I should check how I've got this computer set up
14:31:26 <acozine> we've got four folks actively reviewing right now
14:31:32 <tadeboro> I added my review to the PR. But I think a few more developers/maintainers should go over it and state their oppinion.
14:31:46 <acozine> yeah, this isn't one we can merge today
14:32:06 <acozine> for a little variety, how about we look at some one-liners from the ansible/ansible repo?
14:32:39 <acozine> samccann: are you good to finish out your copyedit review while we look at some other PRs?
14:32:54 <acozine> tadeboro: thanks for the review!
14:33:18 <tadeboro> Yeah, we should probably focus on a more "strictly docs, not a process itself" PRs.
14:33:29 <acozine> #topic https://github.com/ansible/ansible/pull/74334
14:33:36 <samccann> yeah go ahead and move on
14:33:48 <acozine> thx
14:33:59 <acozine> from the conversation on this one, it looks like the original fix is not the right one
14:34:06 <acozine> but the docs are inconsisten
14:34:30 <acozine> s/inconsisten/inconsistent
14:35:11 <acozine> it should either be `3.7` everywhere, or `3.8` everywhere
14:35:36 <tadeboro> I think Matt has this one under control. Not sure what else we can add there.
14:36:25 <acozine> #topic https://github.com/ansible/ansible/pull/74254
14:36:49 <acozine> this one looks good to me
14:37:01 <acozine> we keep making edits to the page and forgetting that some lines are bolded
14:37:21 <acozine> when you add a new line, you have to update the `emphasis` references
14:38:05 * tadeboro is still counting lines ... ;)
14:38:15 <acozine> heh
14:38:42 <tadeboro> Thinks look OK from my side.
14:39:29 <acozine> thanks
14:39:31 <acozine> merged!
14:40:11 <acozine> #topic https://github.com/ansible/ansible/pull/74330
14:40:19 <acozine> is this one really a docs PR?
14:40:43 <acozine> oh, yes, yes it is
14:40:46 <tadeboro> Not sure. Maybe the docs part is not ready yet?
14:41:34 <tadeboro> Oh, I see the docs are already fixed. And it looks like Brian is taking care of this one.
14:41:46 <acozine> hm, well sort of
14:42:46 <tadeboro> Did the PR miss some parts of the docs that need to be updated?
14:42:55 <acozine> that is possible
14:43:46 <Shrews> there is one doc change there
14:43:53 <Shrews> rest are tests
14:43:59 <acozine> welcome Shrews!
14:44:17 <acozine> yeah, just wondering if there are other places we need to update docs
14:44:19 <acozine> to match
14:44:20 <Shrews> i want bcoca to look at that again, so you can skip it
14:45:14 <Shrews> not sure if there are more docs, tbh
14:45:40 <acozine> we've already removed references to the bare `include` param from https://docs.ansible.com/ansible/latest/user_guide/playbooks_reuse.html#re-using-files-and-roles
14:46:14 <bcoca> he, was just starting review
14:46:36 <acozine> hey bcoca
14:46:43 <acozine> thanks!
14:47:06 <acozine> I don't see any references to `include` in the docs about testing
14:47:28 <acozine> so I think the docs are up to date
14:48:20 <acozine> how about this one?
14:48:23 <acozine> #topic https://github.com/ansible/ansible/pull/74288
14:48:29 * samccann finally finished my copyedit from waaay back
14:48:53 <acozine> thanks samccann!
14:49:03 <acozine> ah, this PR is for the Docker guide
14:49:36 <tadeboro> 74288 looks good from the technical side (I had to fix our CI and I know the instructions work as expected).
14:50:07 <acozine> awesome - looks good from the wordsmithing side too
14:50:24 <schurzi> Hi, I think I found a missmatch between Ansible documentation and comments in code. I created a PR to resolve this (https://github.com/ansible/ansible/pull/73211) maybe you can have a look at this
14:50:48 <acozine> schurzi: welcome!
14:50:59 <acozine> #chair schurzi
14:50:59 <zodbot> Current chairs: acozine lmodemal samccann schurzi tadeboro
14:51:07 <acozine> #chair Shrews bcoca
14:51:07 <zodbot> Current chairs: Shrews acozine bcoca lmodemal samccann schurzi tadeboro
14:51:16 <samccann> this line reads funny - https://github.com/ansible/ansible/pull/74288/files#diff-0ea0d8cb54d773b8b4eb8a7ac6421c3a02735af2c2a8c5801ad94c385fefd36aR24
14:51:47 <samccann> `since removed support for Python 2.7` = what removed support for this??
14:51:49 <bcoca> i can kind of understand what they want to state ...
14:51:54 <acozine> ah, good catch
14:52:20 <samccann> did docker remove support? I can add a suggested edit if that's it
14:52:28 <acozine> yeah, that seems right
14:52:33 <acozine> go for it
14:52:40 <tadeboro> Python package for docker removed support for python 2.7.
14:52:53 <samccann> ok I'll add that then, thanks!
14:53:18 <acozine> I think if you add it I can commit it and then we can merge
14:53:33 <samccann> go for it
14:54:34 <acozine> done
14:54:43 <acozine> okay, now let's look at schurzi's PR
14:54:59 <acozine> #topic https://github.com/ansible/ansible/pull/73211
14:55:08 <baptistemm> hello
14:55:23 <samccann> #chair baptistemm
14:55:23 <zodbot> Current chairs: Shrews acozine baptistemm bcoca lmodemal samccann schurzi tadeboro
14:55:25 <samccann> welcome!
14:55:38 <acozine> hey baptistemm!
14:55:43 <samccann> we are running a docs PR today for the next.. erm.. 3 hrs
14:56:15 <acozine> schurzi: can you post a link to the comment that says `_text` is deprecated?
14:56:35 <tadeboro> The changes in 73211 are correct, but I need a minute or two to bring the VNP up and git grep for other occurences.
14:57:11 <acozine> ah, found it: https://github.com/ansible/ansible/blob/devel/lib/ansible/module_utils/_text.py#L7
14:57:45 <schurzi> https://github.com/ansible/ansible/blob/a077bca5d5866603cdc4a2a13aef8f416860aacb/lib/ansible/module_utils/_text.py
14:58:05 <acozine> schurzi: thanks!
14:58:32 <acozine> thank you for noticing the deprecation and the fact that the docs were out of date
14:59:18 <baptistemm> how do you choose PR ? do you use those with doc or doc-only tags ?
15:00:03 <acozine> baptistemm: we're looking at any PRs that folks post here, and also at any PRs in https://github.com/ansible/ansible/pulls?q=is%3Apr+is%3Aopen+label%3Adocs+
15:00:12 <samccann> If you ave a PR you want us to look at, you can bring that up as well
15:00:49 <tadeboro> For 73211, my git grep says it found all occurences.
15:00:49 <acozine> ah, now is our usual time for the DaWGs meeting
15:00:56 <acozine> tadeboro: thanks!
15:01:01 <samccann> woot!  shall I approve and merge?
15:01:06 <acozine> it needs CI
15:01:19 <acozine> so do a `rebuild_merge` on it
15:01:26 <samccann> ok thanks! does that still work?
15:01:37 <samccann> in the azure world? I've forgotten...
15:01:41 <acozine> I think so . . . let's try it and see
15:02:02 <acozine> if it hasn't worked in an hour, we can try a different approache
15:02:07 <acozine> er, approach
15:02:11 <samccann> done. I'll keep the tab open to check
15:02:11 <baptistemm> there is PR of mine https://github.com/ansible/ansible/pull/73984/files if you want
15:02:17 <acozine> baptistemm: sounds great
15:02:29 <schurzi> can you also answer the question on how to handle all the usages in code? I did a only docs Pr because I don't know how to contunie best from here
15:02:38 <samccann> 74288 that had the tiny edit is passing CI now so imma merge it
15:02:57 <acozine> awesome, thanks
15:03:16 <tadeboro> 73211 also has all checks green if I see correctly.
15:03:38 <acozine> tadeboro: yes, but it's marked `stale_ci`
15:03:45 <tadeboro> But they ran a few months ago ...
15:03:53 <tadeboro> I just noticed that, yes.
15:04:11 <acozine> 99 times out of 100 that will be fine, but the 100th time . . .
15:04:15 <samccann> cyb-clock sez we are an hour into this
15:04:22 <acozine> \o/
15:04:33 <acozine> schurzi: to answer your question about the code
15:04:44 <acozine> how many mentions did you find?
15:05:06 <acozine> (of `ansible.module_utils._text`)
15:05:30 <schurzi> in ansible repo a few hundred I think, in all community collections uncountable ...
15:05:34 <acozine> heh
15:05:46 <acozine> here's what I would suggest
15:06:10 <acozine> do a PR that updates at most 25 files
15:06:48 <acozine> you can post it here and in the `ansible-devel` channel
15:06:57 <tadeboro> schurzi: I do not know when exactly that change landed, but I would assume collections cannot be updated right now because they usually support a range of Ansible Core versions.
15:07:24 <samccann> baptistemm - for your PR - there are a few examples that don't have a comment with the expected results, starting here - https://github.com/ansible/ansible/pull/73984/files#diff-a88c795a6652dcd2f429b600ae12d60574d9cc9ac587e43798200a569b8abc17R649
15:07:44 <acozine> #topic https://github.com/ansible/ansible/pull/73984/files
15:07:45 <schurzi> @tade
15:07:45 <samccann> is it not possible to add them? I dunno what permutations etc does...
15:08:06 <schurzi> tadeboro that change is very old ... ansible2.8 or older
15:08:31 <baptistemm> samccann: indeed I added the FQCNs but forgot the examples...
15:08:48 <tadeboro> Then opening PRs in collections is OK since most of then require Ansible >= 2.9
15:08:55 <acozine> dericcrago: dmsimard aminvakil briantist cyberpear kindlehl madonius Tas-sos zbr you folks talking docs today?
15:09:29 <acozine> #info this is the usual start time (well, I'm a bit late) for the Docs Working Group aka DaWGs meeting
15:09:45 <acozine> #info today we are doing a Mini Docs PR Review Day
15:09:56 <schurzi> acozine thanks, i will do that, when I find some time. single PRs per function/plugin seem to be the best approach. Thank you all :)
15:10:08 <samccann> thanks schurzi !!
15:10:32 <acozine> schurzi: thank you! join us any Tuesday at this time for the Docs Working Group
15:10:51 <acozine> and/or chat welcome in the channel any time
15:11:21 <baptistemm> samccann: give me 5 minutes I update that
15:11:44 <acozine> baptistemm: sounds good, post a note when it's ready
15:12:17 <acozine> #topic https://github.com/ansible/ansible/pull/74268
15:12:32 <acozine> this one I find very interesting
15:13:07 <acozine> oh, no, this is a different PR than I thought it was
15:13:09 <acozine> heh
15:13:42 * acozine looks for the PR she was thinking of
15:14:10 <acozine> we should still look at this one, though
15:14:32 <baptistemm> acozine: samccann: I remember, the output was not legible as it outputs all combinations of permutation ... so that why I did not put that
15:15:12 <samccann> ah gotcha thanks
15:15:18 <samccann> I'll approve and merge
15:16:45 <tadeboro> I need to take kiddos out or they will trash our appartment. I will start annoying you all again in an hour or two ;)
15:17:28 <samccann> LOL
15:17:34 <samccann> thanks tadeboro for the help!
15:18:19 <samccann> so on this pr, I wonder if `nsible.module_utils.basic._CHECK_ARGUMENT_TYPES_DISPATCHER` being deprecated is something we need to see if it's used anywhere in the docs as well? or does it sound too arcane to be squirrelled away somewhere?
15:20:09 <samccann> wow.. also, lots of methods being removed and a note about collection owners needing to change. For those of y'all who handle collections, is it enough to have that in the porting guide?
15:20:24 <samccann> wondering if it needs to go out in the bullhorn as well?
15:21:26 <acozine> tadeboro: see you later!
15:22:16 * abadger1999 is back
15:22:21 <acozine> #chair abadger1999
15:22:21 <zodbot> Current chairs: Shrews abadger1999 acozine baptistemm bcoca lmodemal samccann schurzi tadeboro
15:22:31 <acozine> #unchair tadeboro
15:22:31 <zodbot> Current chairs: Shrews abadger1999 acozine baptistemm bcoca lmodemal samccann schurzi
15:22:38 <samccann> hmmm this seems a bit of a flag for us in docsland - https://github.com/ansible/ansible/pull/74268/files#diff-55afa0e374c236bc768e486db6ce391902ba4db29f456b34f7c39fafd9913d3bR47
15:22:40 <acozine> (just while he's out with the kids)
15:23:02 <abadger1999> Hmm... From basic?  Let's ask sdoran .... a few weeks ago, I commented to him that I would leave the old methods in for a looong time but encourage people to use the new things.
15:23:16 <acozine> sphinx 3.2???? goodness, we have some upgrading to do
15:23:36 <abadger1999> sdoran: ^ see samccann's comments about a lot of module_utils methods being deprecated and collection owners needing to update.
15:25:43 <acozine> okay, this PR has a lot more than docs in it
15:26:34 <samccann> yeah just scanned and they already fixed the spelling errors in the code ;-)  That's about all I could have added.
15:26:44 <acozine> heh
15:26:53 <acozine> we should probably add a comment about the sphinx version
15:27:02 <acozine> do we have an issue open about upgrading sphinx?
15:27:27 <samccann> there's probably an old one there somewheres. I can dig it up and put to this PR if you want so we know why it has a new sense of urgency to it ;-)
15:27:40 <acozine> here's the current one: https://github.com/ansible/ansible/issues/74194
15:29:10 <acozine> heh, jinx
15:29:36 <samccann> ok I added a comment.
15:29:59 <samccann> to that issue and in the PR to ask if there will be display problems for 2.11 because we are older for sphinx
15:31:27 <samccann> ...and to consider sending some of the info out in the next bullhorn
15:32:38 <acozine> all right, that might be all we can do on PR 74268 for today
15:32:39 <samccann> anything else on this PR or should we move on?
15:32:48 <acozine> moving on . . .
15:32:56 <acozine> #topic https://github.com/ansible/ansible/pull/74242/files
15:34:50 <samccann> LGTM
15:35:03 <acozine> so if I'm reading this right, the inclusive and exclusive notes mean that random sometimes returns 0 but never returns the upper bound number (for example, 347)
15:35:35 <samccann> yeah. just thinking as well.. dunno if that's a true statement?
15:36:01 <acozine> me neither
15:36:07 <acozine> where's the code?
15:37:27 <acozine> somewhere in here: https://github.com/ansible/ansible/tree/devel/lib/ansible/plugins/filter
15:38:33 * abadger1999 looking
15:39:32 <acozine> is the random number filter ansible-specific? or is it a standard jinja filter?
15:40:04 <abadger1999> I believe it's ansible specific
15:40:42 <samccann> is it? I thought it was using Python systemRandom() ?
15:40:57 <samccann> which does seem to be exclusive on the upper number - https://docs.python.org/3/library/random.html#random.SystemRandom
15:40:57 <acozine> https://jinja.palletsprojects.com/en/2.11.x/templates/#random
15:41:13 <abadger1999> I wonder if we overwrite a jinja filter of hte same name...
15:41:23 <acozine> https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L36
15:41:25 <samccann> https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L36
15:41:25 <acozine> tjat
15:41:29 <samccann> haha JINX
15:41:30 <sdoran> abadger1999: I opened a PR to add those methods back in, but the decision was made to leave them removed. https://github.com/ansible/ansible/pull/74010#issuecomment-809526310
15:41:33 * acozine can't type
15:41:36 <abadger1999> in our docs  it says random is an ansible extension of the jinja2 filter
15:42:07 <sdoran> For what it's worth, I went through all the collections in `ansible-community` and look for the use of private methods and opened issues notifying them of the changes.
15:42:35 <sdoran> (sorry to be jumping in the middle of your current discussion — I was in another meeting earlier)
15:42:41 <samccann> sdoran cool, thanks
15:43:11 <samccann> the other question was the sphinx version needing to be >=3.2. We are at 2.1.2 right now, is that going to cause display problems when we publish 2.11 docs?
15:44:39 <sdoran> samccann: It works as intended with 2.1.2 because I only have a docstring for just that one private attribute.
15:45:44 <sdoran> But since 2.1.2 ignores the parameters, any docstring added to private attributes will be displayed in the rendered doc. But I don't plan to add any docstrings to other private attributes, so it should be fine.
15:46:09 <samccann> k thanks for the explanation
15:46:33 <acozine> for the PR that changes the docs on the  `random` filter, the part about it being an integer is correct based on https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L238
15:47:07 <acozine> I don't know how to evaluate the inclusive/exclusive part of the change
15:49:08 <acozine> oh, and when baptistemm's PR merges, this PR will need rebasing, because they both change the `playbooks_filters.rst` file
15:51:06 <samccann> yeah not sure either how to determin if hte upper bound is exclusive or not
15:52:15 <samccann> is it possible to bring it back to the core team for re-review? they took the triage label off, but dunno if that means they agree and left it to docs to merge, or just looked and said it's docs only?
15:53:46 <acozine> samccann: the docs you posted from the Python systemRandom do show 0 as inclusive and the upper bound as exclusive
15:54:03 <acozine> and the ansible code does specify integers
15:54:18 <samccann> yeah but there's another on where it isn't. https://docs.python.org/3/library/random.html#functions-for-integers
15:55:39 <acozine> we're using the first one (SystemRandom): https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/filter/core.py#L224
15:55:52 <samccann> we are using both
15:56:07 <samccann> SystemRandom and Random. Random seems to be used w/ a seed
15:56:10 <samccann> https://docs.python.org/3/library/random.html#random.Random
15:56:53 <acozine> true
15:57:03 <acozine> there's an example with `seed` lower down
15:57:07 <acozine> in our docs
15:57:20 <acozine> I think this change is good, though
15:57:33 <samccann> ok go for it on the merge then
15:57:56 <samccann> i haven't found my way through the code. But I think you are right it's using SystemRandom (no seed in this example)
15:58:20 <abadger1999> samccann: It is correct about inclusive/exclusive
15:58:35 <acozine> baptistemm: https://github.com/ansible/ansible/pull/73984 is now merged \o/
15:59:29 <samccann> ok merged, thanks abadger1999!
15:59:54 <samccann> cyb-clock sez we are  2 hours into this
16:00:28 <acozine> halfway mark!
16:01:23 <acozine> we're going to do a ten-minute break
16:01:34 <acozine> #topic 10-minute break
16:13:05 <baptistemm> acozine: thanks
16:13:40 <acozine> and we're back!
16:14:02 <acozine> #topic second half of the first-ever Mini Docs PR Review Day
16:14:32 <acozine> I found the PR that I was curious about
16:14:38 <samccann> cool
16:14:40 <acozine> #topic https://github.com/ansible/ansible/pull/74201/files
16:16:11 <acozine> it adds instructions on setting "the structure" for the output you're working on - which I have never done
16:16:13 <acozine> https://github.com/ansible/ansible/pull/74201/files#diff-fdb133ce38964cecbfb4a1ec6fa0d8420eb4dbe627e57f361eab1be9f0cddfb5R125
16:16:20 <samccann> hmm
16:16:36 <acozine> sdoran said he ran into this when building the docs locally
16:16:39 <samccann> yeah that's all part of translation. the foo_structure stuff was added then
16:17:04 <samccann> you do make coredocs or make webdocs
16:17:59 <acozine> but do you also have to do `make ansible_structure` or `make core_structure`?
16:18:01 <acozine> O
16:18:01 <samccann> or really, make webdocs ANSIBLE_VERSION=<version.
16:18:05 <samccann> no
16:18:14 <sdoran> `htmdocs` will run `ansible_structure`
16:18:31 <samccann> I don't know if something is now doing that under the covers. I never tested what the structure stuff does. I just know it was added by the translation PR
16:18:33 <sdoran> But in my case, I was just wanting to run `htmlsingle` which complained because no `conf.py` was present.
16:18:37 <acozine> ahhhh
16:18:57 <sdoran> So I figured out one of the structure commands needs to be run first to symlink in the correct config.
16:18:58 <samccann> ah yeah, forgot about that one!
16:19:06 <acozine> so it's necessary if you're building a single page and you haven't build the whole docs at least once beforehand
16:19:08 <acozine> that makes sense
16:19:14 <samccann> yes and no
16:19:27 <samccann> what we really need to do is update the makefiles so htmlsingle can run w/ the correct conf.py
16:19:43 <samccann> we probably also need to test/fix who to run docs w/ just one module
16:19:55 <acozine> would we set core as the default, then?
16:20:13 <sdoran> And would it be slow to run the structure target every time?
16:20:17 <samccann> dunn. haven't thought about it yet
16:20:18 <sdoran> `htmlsingle` is nice because it's fast
16:20:33 <acozine> yeah, I use it a lot
16:20:51 <samccann> core doesn't include all the docs so I don't think core would be the default
16:21:33 <samccann> ansible includes ALMOST all the docs, but again... not all of them.
16:21:34 <acozine> I meant the default for "I'm just building a single page"
16:21:43 <acozine> ah, right
16:21:47 <samccann> again, not without changes
16:22:33 <samccann> my quick nickel is we open an issue to say htmlsingle and most like the one that will build just one module doc is likely broken. Point to this PR for a 'short term workaround' but not merge the PR
16:22:35 <acozine> what would happen if I tried to use `htmlsingle` with an rst file that wasn't included in the core build, after running the full core docs build?
16:22:54 <samccann> dunno never tried it
16:23:08 <samccann> I'd guess it wouldn't work becfause the file is excluded by sphinx in the conf.py file
16:23:12 <acozine> sdoran: do you remember what error you ran into?
16:24:04 <acozine> we could include the extra instructions in this PR as troubleshooting
16:24:21 <sdoran> Sphinx errorer saying no `conf.py` existed.
16:24:34 <acozine> sdoran: thanks
16:24:52 <sdoran> Just do `make clean` then `make htmlsingle`.
16:25:05 <sdoran> Granted `make clean` is currently broken and won't remove the symlink.
16:25:09 <sdoran> That's the other fix in that PR.
16:25:32 <samccann> yeah what I could do is take over this PR, figure out the makefile changes etc, and also update the docs section based on that?
16:25:56 <sdoran> Sounds good.
16:26:46 <samccann> #action samccann to take over https://github.com/ansible/ansible/pull/74201 and update makefiles etc so htmlsingle and making docs w one module works again
16:27:09 <acozine> sdoran: ah, that explains why I never ran into this error
16:27:23 <acozine> I'm glad you did, because you will not be the only one, I'm sure
16:29:15 <acozine> #topic https://github.com/ansible/ansible/pull/74161
16:29:23 <acozine> finally! an easy one!
16:29:50 <acozine> let me just double-check that we can merge backports to 2.9 today
16:30:35 <samccann> cool! gave it an approval
16:31:21 <acozine> #topic https://github.com/ansible/ansible/pull/74158/files
16:33:10 <acozine> oops, I have tea steeping
16:33:11 <acozine> BRB
16:33:17 <samccann> heh
16:34:53 <acozine> okay
16:35:03 <acozine> I have some suggestions for this PR
16:35:30 <samccann> yeah i'm making some as well
16:36:40 <samccann> ok submitted my suggested edits
16:36:52 <samccann> but not sure what the last sentence fragment means or how to fix it.
16:41:40 <samccann> ok do we batch and commit comments and then merge? or is there anything controversial we should wait on?
16:43:00 <acozine> bcoca: you around?
16:43:20 <bcoca> no
16:43:28 <samccann> heh
16:44:36 <bcoca> i'll look after meetinsg
16:44:45 <acozine> okay, thanks
16:44:45 <bcoca> but feel free to merge with fixes
16:45:40 <acozine> #topic https://github.com/ansible/ansible/pull/74129/files
16:45:45 <acozine> this one looks simple
16:47:16 <acozine> I vote we add a note saying "thanks, did this example trigger an error messages for you? if so, please post it here so other users can find your fix"
16:48:07 <samccann> I can't tell if it's right or not. I mean the code is there
16:50:06 <acozine> me neither
16:50:27 <samccann> hehe well that was a circular journey - went to the kubernetes collection, found the plugin, clicked on teh docs.. and wEEEE back to the same page we are looking at ;-)
16:50:46 <acozine> heh
16:51:19 <acozine> akasurde assigned himself to the PR, I expect he'll test it
16:51:26 <samccann> ok cool
16:52:32 <acozine> #topic https://github.com/ansible/ansible/pull/74105/files
16:52:55 <acozine> oof, it needs a rebase
16:53:13 <acozine> I can do that quickly
16:53:49 <samccann> cool
16:53:54 <samccann> i'll reread whilst you do that
16:53:59 <acozine> thanks
16:59:31 <acozine> hrm
16:59:36 <acozine> this rebase is going to get complicated
16:59:58 <samccann> ok do we wait on it and I can review/merge later?
17:00:08 <samccann> cyb-clock says we are 3 hours into the meeting
17:00:15 <samccann> and possiblly talking to ourselves at this point?
17:00:37 <samccann> so I'm game either way
17:00:46 <tadeboro> Hi. I am back for round two.
17:01:42 <samccann> welcome back!
17:01:46 <samccann> #chair tadeboro
17:01:46 <zodbot> Current chairs: Shrews abadger1999 acozine baptistemm bcoca lmodemal samccann schurzi tadeboro
17:02:14 <acozine> samccann: let me know what you think of the current state of that PR
17:02:40 <acozine> hey tadeboro
17:02:50 <acozine> we're looking at https://github.com/ansible/ansible/pull/74105/files
17:02:55 <samccann> tadeboro - any chance you know if this one is correct? https://github.com/ansible/ansible/pull/74129
17:02:58 <samccann> heh woopsie
17:03:02 <samccann> BAM BAMABAM!
17:03:04 <acozine> heh
17:03:07 <acozine> we can triple-task
17:03:37 * tadeboro looking
17:04:29 <samccann> acozine I'm happy with the dev guide split. just waiting on the rebase
17:06:26 <tadeboro> The 74105 looks correct. Upstream docs with a relevant example: https://kubernetes.io/docs/concepts/workloads/pods/#pod-templates
17:07:32 <schurzi> I'm leaving. From my point of view this PR day was a great idea and the experience as a first time contributor was also great. Thank you all!
17:08:35 <acozine> schurzi: thanks for coming!
17:10:37 <acozine> tadeboro: thanks for reviewing 74105
17:11:01 <acozine> er, sorry, 74129
17:12:24 <acozine> heh, and we have new docs PRs coming in even as we work!
17:12:30 <acozine> https://github.com/ansible/ansible/pull/74348 looks like a good addition
17:13:00 <samccann> #topic https://github.com/ansible/ansible/pull/74088
17:13:57 <samccann> LGTM gonna merge it
17:14:15 <acozine> heh, jinx
17:14:18 <samccann> HAH
17:14:25 * samccann hands acozine her soda back
17:15:50 <acozine> #topic https://github.com/ansible/ansible/pull/74076
17:16:32 <acozine> the security objection seems sound to me
17:16:41 <samccann> yeah sorry forgot to close that one out
17:17:07 <acozine> excellent! another PR Day win!
17:19:00 <acozine> #topic https://github.com/ansible/ansible/pull/74087
17:19:04 <acozine> I'd also vote to close this one
17:20:12 <tadeboro> Agreed.
17:20:23 <samccann> maybe close with a note to say consider opening another PR with a loop example instead?
17:21:21 <tadeboro> Looping over services is not something one would do often.
17:21:51 <acozine> samccann: ah, could you add a comment to that effect?
17:22:05 <samccann> ah.. then maybe just close it
17:22:05 <acozine> I closed it already, but the OP will continue to see comments, I think
17:22:18 <acozine> we're at 100 open docs PRs
17:22:24 <acozine> one more and we're back in double-digits
17:22:31 <samccann> woot!!
17:22:38 <acozine> in the ansible/ansible repo, at least
17:23:21 <acozine> #topic https://github.com/ansible/ansible/pull/74088
17:23:30 <acozine> kindlehl put a review on this one already (thanks!)
17:25:07 <tadeboro> I also read that one before and it is technically correct.
17:25:54 <samccann> yeah I'm trying to find evidence on whether the collection has been released with this new platform
17:26:29 <samccann> I can see it in the repo from 20 days ago, and it was just updated on galaxy I think 5 days ago?
17:26:45 <samccann> good enough to say yah it's been released?
17:27:06 <acozine> yeah, that seems good
17:28:04 <samccann> ok I'll approve and merge
17:28:15 <tadeboro> I think that part was not released yet.
17:28:16 <acozine> \o/
17:28:22 <acozine> tadeboro: what part?
17:28:42 <tadeboro> https://github.com/ansible-collections/community.network/pull/111 is not part of a release yet.
17:28:42 <samccann> how can you tell since it was in the repo 20 days ago?
17:29:56 <samccann> ah it's not on the cliconf plugins in Galaxy, I think you're right!
17:30:01 <acozine> ahhh
17:30:12 <acozine> so we're waiting on the next release of community.network
17:30:24 <tadeboro> This is a new feature that will be part of the 2.2.0
17:30:38 <tadeboro> Patch releases cannot add new features.
17:31:19 <acozine> which version will go into Ansible 4?
17:31:19 <samccann> gotcha
17:31:21 <samccann> phew
17:31:37 <acozine> do we know yet?
17:32:01 <samccann> anything up to but not including 3.0.0
17:32:18 <samccann> https://github.com/ansible-community/ansible-build-data/blob/main/4/ansible-4.build#L40
17:32:30 <acozine> we can put a note on the ansible/ansible PR that says "merge when community.network 2.2.0 is released"
17:32:39 <acozine> thanks for the link samccann
17:32:39 <samccann> you thinking we merge because it's supposed to be part of Ansible 4 and this is devel?
17:32:49 <acozine> we could, or we could wait
17:32:49 <tadeboro> If 2.2.0 is released before Ansible 4.0.0, then yes, else no.
17:33:01 <acozine> gotcha
17:33:06 <acozine> then let's put a note on it and wait
17:33:08 <samccann> this is another reason we need these guides punted from ansible/ansible ;-)
17:33:11 <samccann> will do
17:33:32 <acozine> heh, though I guess devel is now 2.12, really, which will be Ansible 5 . . . but let's wait anyway
17:33:47 <tadeboro> Moving scenario guides to collections will be one big win for everyone involved.
17:34:29 <samccann> acozine yeah I thought about that, but unlik devel for ansible/ansible, this functionality isn't actually available even if they download from galaxy
17:34:43 <acozine> yes and yes
17:34:58 <samccann> cyb-clock sez we are 3.5 hrs into our 4 hr pr marathon
17:34:59 <acozine> #topic https://github.com/ansible/ansible/pull/73716/files
17:35:10 <acozine> One. More. Merge
17:35:46 <samccann> this one passed CI  want me to merge? https://github.com/ansible/ansible/pull/74129
17:36:07 <samccann> I think rebuild_merge will eventually, but you know... if you want that coveted double digit...
17:36:12 <acozine> heh, sure
17:36:14 <acozine> go for it
17:37:19 <samccann> oh bonus points! this new PR links to ansible 2.4 docs!
17:38:00 <acozine> samccann: which one?
17:38:30 <samccann> 73716
17:38:35 <samccann> the one we are reviewing now
17:38:41 <acozine> oh, in the summary
17:38:45 <acozine> heh, I'd missed that
17:38:56 <tadeboro> https://docs.ansible.com/ansible/latest/reference_appendices/config.html#default-allow-unsafe-lookups is the "modern" alternative with the same info.
17:40:42 <acozine> yeah, I'm gonna edit the summary
17:40:48 <acozine> we don't need more references to 2.4
17:41:00 <acozine> otherwise, though, what do folks think of the change?
17:41:12 <tadeboro> The wording in that PR is a bit off since it is not the lookup itself that is marked as unsafe, its return value is.
17:41:46 <abadger1999> Regarding 74201, I didn't know about htmlsingle..... there's also singlehtml....
17:42:00 <acozine> link updated
17:42:09 <samccann> 'lookup return values are marked unsafe..." ?
17:42:20 <samccann> as in a suggested edit, then we commit and merge?
17:42:23 <tadeboro> Yes, that is the correct variant.
17:42:53 <acozine> +1
17:43:12 <acozine> who wants to make the suggested edit?
17:43:15 <samccann> did it
17:43:21 <samccann> you wanna commit and merge?
17:43:25 <acozine> \o/
17:44:09 <acozine> committed
17:44:17 <acozine> as soon as CI runs, I'll merge
17:45:32 <abadger1999> sdoran: What does htmlsingle do?
17:46:21 <acozine> it builds one rst page into html
17:46:51 <acozine> you pass it `rst=path/to/file.rst`
17:47:14 <abadger1999> Ahh, so not even a single plugin... it's a single file.
17:47:34 <acozine> yep
17:47:36 <abadger1999> (And you just mentally ignore all the undefined reference warnings in the output)?
17:47:40 <acozine> yeah
17:47:47 <acozine> it's handy when you're testing changes
17:47:53 <abadger1999> Okay.  I think I can help out with the Makefile change.
17:47:59 <acozine> abadger1999: awesome!
17:48:07 <abadger1999> I'll post a comment to the PR for samccann  when I do.
17:48:24 <samccann> cool thanks abadger1999
17:49:13 <acozine> okay, we have ten minutes left
17:50:36 <sdoran> Yup. Just builds a single rst file into HTML.
17:54:08 <samccann> we gonna try for one more pr?
17:55:06 <acozine> I merged 73716
17:55:10 <samccann> oh wow... the search we used here doesn't include all of these -https://github.com/ansible/ansible/pulls?q=is%3Aopen+is%3Apr+label%3Adocs_only
17:55:19 <samccann> as in some are just getting docs_only labels
17:55:31 <acozine> hm
17:55:55 <samccann> sorry this is a better one - https://github.com/ansible/ansible/pulls?q=is%3Aopen+is%3Apr+label%3Adocs_only+-label%3Adocs
17:56:06 <samccann> only 4 have the `docs_only` but no `docs` label
17:56:18 <tadeboro> https://github.com/ansible/ansible/pull/73273/files looks like an easy one.
17:56:27 <acozine> yeah, that's a trend we need to think about - is that something we want?
17:56:56 <samccann> #topic https://github.com/ansible/ansible/pull/73273/files
17:56:57 <acozine> tadeboro: nice
17:57:45 <acozine> what do we think about baptistemm's comment about making those references links?
17:57:50 <samccann> does it? haha
17:58:37 <samccann> i don't know if sphinx makes them links or not. I'd guess it does
17:58:41 <acozine> I can add suggestions easily enough
17:58:46 <acozine> I don't think so, no
17:59:15 <tadeboro> I would say we convert those to explicit links and commit the changes.
17:59:22 <samccann> yeah sounds good
17:59:40 <samccann> acozine you wanna do it and I'll merge this time?
18:00:51 <acozine> https://github.com/ansible/ansible/pull/73273#pullrequestreview-640276535
18:01:06 <acozine> samccann: go for it
18:02:26 <samccann> ok waiting on CI
18:02:34 <samccann> and let's count how many PRs we've done
18:03:50 <samccann> 11 PRs merged, 7 additional PRs reviewed
18:04:01 <samccann> tho one of those will go to the merged pile soon!
18:04:14 <acozine> I count 12
18:04:21 <samccann> #info merging 12 PRs and reviewed additional 7 PRs
18:04:34 <samccann> not bad... not bad at all
18:04:57 <tadeboro> To what pile the closed ones go? Or was just one that we closed without merging?
18:05:58 <samccann> ah sorry yeah the 'merged' included one closed PR
18:06:06 <samccann> #undo
18:06:06 <zodbot> Removing item from minutes: INFO by samccann at 18:04:21 : merging 12 PRs and reviewed additional 7 PRs
18:06:22 <samccann> #info 11 PRs merged, 1 closed, 7 reviewed/requested changes
18:06:33 <samccann> ok we ready to close this meeting out?
18:06:42 <acozine> I think so
18:06:47 <acozine> I need some lunch!
18:07:12 <acozine> thanks samccann, abadger1999 and tadeboro for sticking around to the end!
18:07:31 <acozine> Thanks everyone who brought a PR to the group, and to everyone who reviewed a PR
18:07:52 <acozine> please post any suggestions on how to improve the experience to the chat
18:07:57 <acozine> #endmeeting