16:23:48 <abadger1999> #startmeeting ansible core public irc meeting (part deux)
16:23:48 <zodbot> Meeting started Thu Jun 11 16:23:48 2020 UTC.
16:23:48 <zodbot> This meeting is logged and archived in a public location.
16:23:48 <zodbot> The chair is abadger1999. Information about MeetBot at http://wiki.debian.org/MeetBot.
16:23:48 <zodbot> Useful Commands: #action #agreed #halp #info #idea #link #topic.
16:23:48 <zodbot> The meeting name has been set to 'ansible_core_public_irc_meeting_(part_deux)'
16:24:03 <abadger1999> #topic tombstoning
16:24:04 <felixfontein> the code uses removed_in/removed_at_date, ignores redirect; the actual ansible_builtin_runtime.yml contains warning_text which isn't mentioned in the code at all
16:24:41 <felixfontein> https://github.com/ansible/ansible/blob/devel/lib/ansible/plugins/loader.py#L448-L459
16:24:56 <felixfontein> vs
16:24:57 <felixfontein> https://github.com/ansible/ansible/blob/devel/lib/ansible/config/ansible_builtin_runtime.yml#L2163-L2166
16:24:59 <nitzmahone> hrm, that might've gotten lost in one of several merge conflict resolutions in the stuff you were changing in there ;)
16:25:09 * nitzmahone looks
16:25:25 <felixfontein> I was quite confused when I was trying to create a validation schema for it
16:25:28 * nitzmahone was getting really good at merge conflict resolution in that code
16:25:35 <felixfontein> heh, no surprise :D
16:26:24 <felixfontein> hmm, I think the redirect was actually in an older version of the plugin loader
16:26:43 <nitzmahone> Yeah, the warning_text for tombstone should behave the same as for deprecations
16:26:45 <felixfontein> (there was a `redriect = tombstone.get('redirect')` that was never used again)
16:26:58 <felixfontein> hmm, now I see that it was mainly the old branch I was looking at :/
16:27:02 <nitzmahone> yeah, that one was definitely a merge conflict artifact :(
16:27:27 <nitzmahone> (or just a vestige of an old local version)
16:27:40 <felixfontein> ok. in that case it's pretty clear: there must be removal_version and removal_date (mutually exclusive and one required?) and warning_text (optional)
16:27:45 <nitzmahone> yep
16:27:52 <abadger1999> #chair felixfontein nitzmahone
16:27:52 <zodbot> Current chairs: abadger1999 felixfontein nitzmahone
16:27:55 <felixfontein> sounds good. I'll add that to gundalow's PR
16:28:19 <felixfontein> how about accepting YAML dates?
16:28:24 <nitzmahone> I had a local rewrite of his schema that implemented that in a different way, but I haven't had time to go back to it
16:28:27 <felixfontein> right now gundalow's PR wants strings for removal_date
16:28:41 <abadger1999> #info there must be removal_version and removal_date (mutually exclusive and one required?) and warning_text (optional)
16:28:59 <nitzmahone> YAML dates would be preferable for the schema- that was the first thing I noticed as well.
16:29:41 <felixfontein> YAML dates are not accepted in module docs (by validate-modules)
16:29:48 <nitzmahone> It'd be good to have a valid format for future sanity tests like "hey, this thing has passed its removal date:
16:30:52 <felixfontein> if YAML dates are ok, I can add something that either expects a valid ISOxxx string (always forget the number) or a Python date object (parsed from the YAML date - or a datetime object, would have to check)
16:30:52 <bcoca> incorporate into the RM script that checks derpecated version now
16:31:05 <nitzmahone> Either string YYYY-MM-DD or YAML date, I'd been using the latter in the internal stuff, but so long as we can parse it to a datetime, I don't have strong feelings either way
16:31:25 <felixfontein> bcoca: I don't know that one, but I guess that one could also be updated later after freeze
16:31:36 <nitzmahone> yep
16:31:56 <nitzmahone> bcoca: best typo of the day, BTW :D
16:31:59 <cyberpear> :w
16:32:01 <cyberpear> :w
16:32:01 <felixfontein> ok. I'll update gundalow's PR for runtime.yml validation, and I'll adjust validate-modules
16:32:03 <bcoca> felixfontein: ping sivel
16:32:10 * nitzmahone needs to start using `derprecated` more often
16:32:19 <bcoca> nitzmahone: who said it was typo?
16:32:20 <abadger1999> https://github.com/ansible/ansible/blob/devel/hacking/build_library/build_ansible/command_plugins/file_deprecated_issues.py
16:32:24 <nitzmahone> bcoca: :D
16:33:00 <felixfontein> abadger1999: I guess that script has to be adjusted anyway to updated ansible-test output
16:33:01 <abadger1999> felixfontein: ^ that's the RM script
16:33:28 <abadger1999> Likely, yeah.  sivel wrote it.  I wrote the framework it lives within.
16:33:59 <felixfontein> also it's not that important anymore, at least I'd hope there are a lot less deprecations in ansible-base now than there used to be in ansible/ansible with all plugins/modules in it :)
16:35:50 <bcoca> dates were meant mostly for collections, specifically AH ones since its support issue
16:36:01 <bcoca> just doesnt work 'like you'ld think' in that case
16:36:59 <abadger1999> so... is this an agreed now?  felixfontein if so, could you summarize in a #agreed?
16:37:16 <acozine> do we have a way forward for 69796 to get merged before the beta is cut?
16:37:23 <acozine> heh, what abadger1999 said
16:37:46 <felixfontein> #agreed dates in YAML can either be a YAML date or YYYY-MM-DD (ISO xxxx) - that's for plugin/module documentation, meta/runtime.yml, and its ansible-base equivalent
16:38:11 <felixfontein> acozine: I think it can be merged now, if bcoca is happy
16:38:21 <abadger1999> #topic Getting 69796 merged before beta
16:38:28 <felixfontein> acozine: btw, do you want to merge 69968?
16:38:37 * acozine looks
16:38:49 <felixfontein> (that's your changelog categories PR)
16:39:13 <abadger1999> felixfontein: Cool, so is the agreement here that we agreed that 69796 is ready to merge?
16:39:22 <abadger1999> felixfontein: or is there more work to be done now?
16:39:30 <felixfontein> abadger1999: I don't know if anyone else sees it this way :)
16:39:34 <felixfontein> bcoca: what do you think?
16:39:48 <felixfontein> I think that specific PR is done
16:39:59 <felixfontein> having a new PR which shows the FQCN in ansible-doc man output would be nice
16:40:13 <felixfontein> no idea who wants to do that though (I'm not sure I have time for it)
16:40:24 <acozine> I'm in favor of merging 69968 but I won't/don't merge my own PRs
16:40:43 <acozine> I'm also in favor of merging 69313
16:40:48 <felixfontein> acozine: I unfortunately can't merge anymore... :)
16:40:52 <abadger1999> <nod>  as long as that doesn't affect the --json output, that seems reasonable to me to go in after beta.
16:41:04 <felixfontein> I think 69313 is ready, mattclay probably wants a last look though
16:41:15 <felixfontein> (it's the antsibull-changelog integration PR)
16:41:36 <felixfontein> abadger1999: yep, could be seen as a bugfix
16:41:36 <mattclay> Yes, I'm doing final review on that today.
16:41:37 <acozine> yeah, those two go together, even though they don't depend on each other in the technical sense
16:41:51 <felixfontein> acozine: sounds good!
16:41:56 <felixfontein> mattclay: great, thanks!
16:42:44 <acozine> mattclay: thanks! if you merge it can you also merge 69968, which updates the 2.10 changelog to use the same categories as the collections changelogs?
16:43:08 <acozine> (by calling out `security_fix` changes)
16:44:16 <abadger1999> #agreed 69313 is going through final review for merging today
16:44:28 <abadger1999> #action mattclay is doing final review on 69313
16:45:02 <abadger1999> #info a followup PR to change how ansible-doc displays name in its human-text output would be appropriate for after beta (but not --json output)
16:45:15 <felixfontein> I guess breaking_changes will get its first proper use for 2.11 then, which is fine for me
16:45:16 <abadger1999> #undo
16:45:16 <zodbot> Removing item from minutes: INFO by abadger1999 at 16:45:02 : a followup PR to change how ansible-doc displays name in its human-text output would be appropriate for after beta (but not --json output)
16:45:24 <abadger1999> #info a followup PR to change how ansible-doc displays name as FQCN in its human-text output would be appropriate for after beta (but not --json output)
16:45:27 <mattclay> acozine: Yeah. They're related because of the changelog category cahnges?
16:45:35 <acozine> mattclay: yes
16:46:13 <felixfontein> mattclay: they are kind of independent, since both the old and the new changelog generator support an arbitrary list of sections, but this list of sections is the same as collections should use, and docs team wanted that for ansible-base as well
16:47:08 <abadger1999> mattclay, nitzmahone : I'll do a quick look and merge https://github.com/ansible/ansible/pull/69968 after the meetin if there's no objections from you.
16:47:13 <acozine> it would allow us to have a security updates page/list in future, and meanwhile it calls out the CVEs in the main changelog
16:47:32 <abadger1999> #action abadger1999 to look at https://github.com/ansible/ansible/pull/69968 and merge (acozine wrote and felixfontein approved)
16:47:33 <nitzmahone> abadger1999: thanks, works for me
16:47:41 <mattclay> abadger1999: Thanks
16:48:25 <felixfontein> great!
16:48:31 <abadger1999> So what are we blocked on... just need bcoca to confirm that he's now fine with https://github.com/ansible/ansible/pull/69796 ?
16:50:53 <felixfontein> I guess so
16:51:02 <abadger1999> felixfontein: I just see the one comment from him in the PR which he said he agreed with everyone else earlier.  Is that the only thing you know about?
16:51:20 <felixfontein> abadger1999: he wanted to check about the FQCN for the `vars` test plugin
16:51:33 <felixfontein> because he wanted FQCNs in all plugins, but that plugin actually didn't have one
16:51:47 <felixfontein> I noticed after adding FQCNs back - and was surprised that I added one back that hasn't been there before
16:52:22 <abadger1999> <nod> So reverting that to the shortname? ( https://github.com/ansible/ansible/pull/69796#pullrequestreview-429068229  )
16:53:07 <felixfontein> whatever bcoca wants
16:53:19 <abadger1999> I see bcoca approved the PR
16:53:28 <abadger1999> (just now)
16:53:43 <acozine> that's a step forward
16:53:59 <felixfontein> cool!
16:54:52 <abadger1999> So I think from the discussion, we were agreed that should be the shortname.
16:55:02 <acozine> yes
16:55:11 <felixfontein> true. but let's fix that for all of the test plugins in the follow-up PR
16:55:23 <abadger1999> Okay, works for me.
16:55:25 <felixfontein> because right now all other plugins in that test collection use FQCN
16:55:55 <abadger1999> as long as the test itself is good and it's jsut the test data that needs to change, it sounds fine for post-beta.
16:56:05 <acozine> how many plugins are we talking about?
16:56:12 <acozine> 10, 100, 1000?
16:56:17 <felixfontein> acozine: < 10
16:56:20 <felixfontein> I think 4-5
16:56:32 <felixfontein> (in the ansible-doc integration test)
16:56:52 <acozine> I'm happy to change those to use shortname and open a PR for that
16:57:03 <abadger1999> #action abadger1999 will merge  https://github.com/ansible/ansible/pull/69796 [ felixfontein and bcoca now think it is ready]
16:57:25 <abadger1999> #action acozine will update the ansible-doc integration test to use shortname
16:57:32 <felixfontein> acozine: feel free! there might be tests failing, but they should be easy to fix
16:57:48 <abadger1999> #topic open floor
16:57:57 <cyberpear> https://github.com/ansible/ansible/pull/70007
16:58:01 <abadger1999> felixfontein, etc: anything else we're missing that needs discussing before beta goes out?
16:58:08 <cyberpear> (hopefully uncontroversial)
16:58:26 <abadger1999> #topic https://github.com/ansible/ansible/pull/70007  config: singular ANSIBLE_COLLECTIONS_PATH
16:58:27 <cyberpear> the gist of it is https://github.com/ansible/ansible/pull/70007/commits/a44689b1aeaef1aa0a4eab5f7be0dccdabadf15a
16:58:30 <abadger1999> nitzmahone: ^
16:59:10 <cyberpear> added the other 2 commits per request from bcoca and felixfontein but they  aren't critical, IMO, if you want the simplest possible change to review.
16:59:27 <felixfontein> I have no idea whether version_added inside there actually works though
17:00:01 <cyberpear> it's there for some others, so it doesn't break anything
17:01:31 <cyberpear> when updating the references in the followup commits, I found some had already erroneously referred to the then-nonexistent singular name :P
17:01:49 <cyberpear> I've tripped on it enough times I finally sent the PR...
17:02:50 <felixfontein> I like the singular variant
17:02:58 <nitzmahone> Yeah, I'm fine with that- I think I waffled on it a few times myself
17:02:59 <acozine> from a docs perspective, this makes a ton of sense - set a pattern, stick to it
17:03:15 <abadger1999> Cool
17:03:48 <abadger1999> nitzmahone: Does that need another review or should I just sanity check it and then merge it?
17:04:01 <nitzmahone> I *almost* did that with the collections plugins dir names in the original 2.8 (singularizing most/all), but it caused a lot of problems, so I left the inconsistent names we used internally. :(
17:04:27 <nitzmahone> abadger1999: the only thing would be to validate with bcoca (or the code) that the preferred entry is in the right list position
17:04:29 <cyberpear> yeah, I left the internal name of the setting alone
17:04:36 <nitzmahone> IIRC the way it is right now, it actually prefers the old name
17:04:43 * cyberpear looks
17:04:45 <nitzmahone> (but I don't remember what order it applies them in)
17:05:06 <cyberpear> last one wins
17:05:13 <nitzmahone> OK, so it's correct as-is then
17:05:18 <nitzmahone> thanks for checking ;)
17:05:21 <abadger1999> Cool.
17:05:26 <nitzmahone> +1 then
17:05:28 * cyberpear has done this change before
17:05:40 <abadger1999> #agreed 70007 is ready to merge (+1 from nitzmahone)
17:05:53 <abadger1999> #action abadger1999 will merge after the meeting.
17:05:56 <cyberpear> thanks!
17:06:01 <felixfontein> as long as the old form is kept working, I'm happy!
17:06:04 <abadger1999> Okay, anything else?
17:06:10 <felixfontein> (for now, not forever)
17:06:25 <abadger1999> I will close in 10 seconds unless someone hollers wait.
17:06:50 <abadger1999> #endmeeting