| *** persia is now known as Guest285 | 00:19 | |
| *** rlandy is now known as rlandy|out | 00:29 | |
| ianw | really can't tell the difference between the working issue request and the failing one | 00:58 |
|---|---|---|
| ianw | Mon Nov 28 23:55:46 UTC 2022] Dns record not added yet, so, save to /etc/letsencrypt-certs/docs.airshipit.org_ecc/docs.airshipit.org.conf and exit. | 01:03 |
| ianw | hrm ... i wonder if it's writing out the tokens into /etc/letsencrypt-certs/docs.airshipit.org_ecc/docs.airshipit.org.conf but then reading /etc/letsencrypt-certs/docs.airshipit.org/docs.airshipit.org.conf | 01:05 |
| ianw | so when we renew, it doesn't see those token | 01:05 |
| clarkb | its odd it would work sometimes if that is the case? | 01:12 |
| *** clarkb is now known as Guest298 | 01:19 | |
| *** Guest298 is now known as clarkb | 01:20 | |
| ianw | on the issue -> [Mon Nov 28 23:55:45 UTC 2022] DOMAIN_PATH='/etc/letsencrypt-certs/docs.airshipit.org_ecc' | 01:31 |
| ianw | on the renewal -> [Mon Nov 28 23:58:00 UTC 2022] DOMAIN_PATH='/etc/letsencrypt-certs/docs.airshipit.org' | 01:32 |
| ianw | almost certainly has to do with https://github.com/acmesh-official/acme.sh/commit/ec0e871592d286b8cd4e1d407411f0ba17c775e6 | 01:32 |
| ianw | i've filed https://github.com/acmesh-official/acme.sh/issues/4416 | 02:04 |
| ianw | i think this works when we re-run it, because then it's using the tokens from the last run | 02:05 |
| ianw | i will pin static to 3.0.5 and run it manually, my intuition is that it will all work | 02:05 |
| ianw | yep | 02:09 |
| *** atmark is now known as Guest305 | 02:10 | |
| opendevreview | Ian Wienand proposed opendev/system-config master: letsencrypt: pin acme.sh to 3.0.5 https://review.opendev.org/c/opendev/system-config/+/865930 | 02:33 |
| opendevreview | Ian Wienand proposed opendev/system-config master: letsencrypt: more echos in driver output https://review.opendev.org/c/opendev/system-config/+/865936 | 03:56 |
| opendevreview | Ian Wienand proposed opendev/system-config master: letsencrypt: add variable for more verbose debugging https://review.opendev.org/c/opendev/system-config/+/865937 | 03:56 |
| *** yadnesh|away is now known as yadnesh | 04:14 | |
| *** pojadhav|out is now known as pojadhav|ruck | 04:51 | |
| frickler | regarding the possible "use gitea for issues" discussion I think we also need to start the discussion on how we want to position ourselves between the original gitea and what seems to be becoming a major fork https://codeberg.org/forgejo/forgejo | 06:27 |
| frickler | fwiw I tried yesterday to look at how codeberg does scale their gitea deployment, which does use issues and PRs and things already, but it seems they don't, running single-server so far | 06:28 |
| *** ysandeep is now known as ysandeep|afk | 08:02 | |
| *** yadnesh is now known as yadnesh|afk | 08:11 | |
| *** jpena|off is now known as jpena | 08:23 | |
| *** yadnesh|afk is now known as yadnesh | 08:27 | |
| opendevreview | Ian Wienand proposed opendev/system-config master: letsencrypt: more echos in driver output https://review.opendev.org/c/opendev/system-config/+/865936 | 09:09 |
| opendevreview | Ian Wienand proposed opendev/system-config master: letsencrypt: add variable for more verbose debugging https://review.opendev.org/c/opendev/system-config/+/865937 | 09:09 |
| *** ysandeep|afk is now known as ysandeep | 09:36 | |
| opendevreview | Cedric Jeanneret proposed opendev/base-jobs master: Add env var for ansible-galaxy proxy https://review.opendev.org/c/opendev/base-jobs/+/865970 | 10:38 |
| *** rlandy|out is now known as rlandy | 11:05 | |
| *** dviroel|afk is now known as dviroel | 11:12 | |
| *** frenzy_friday is now known as frenzy_friday|rover | 12:15 | |
| opendevreview | Merged openstack/project-config master: Use kolla.config for kolla-ansible in gerrit https://review.opendev.org/c/openstack/project-config/+/865686 | 12:50 |
| opendevreview | Cedric Jeanneret proposed opendev/base-jobs master: Add env var for ansible-galaxy proxy https://review.opendev.org/c/opendev/base-jobs/+/865970 | 13:04 |
| *** dasm|off is now known as dasm | 13:05 | |
| *** ysandeep is now known as ysandeep|afk | 13:16 | |
| opendevreview | Merged openstack/project-config master: Ensure NetworkManager doesn't override /etc/resolv.conf https://review.opendev.org/c/openstack/project-config/+/865433 | 13:19 |
| *** pojadhav|ruck is now known as pojadhav|out | 13:32 | |
| *** frenzy_friday|rover is now known as frenzy_friday|rover|food | 13:43 | |
| *** ysandeep|afk is now known as ysandeep | 15:20 | |
| *** dviroel is now known as dviroel|lunch | 16:11 | |
| *** ysandeep is now known as ysandeep|out | 16:20 | |
| *** frenzy_friday|rover|food is now known as frenzy_friday|rover | 16:21 | |
| opendevreview | Clark Boylan proposed zuul/zuul-jobs master: Add support for nodejs testing on Debian https://review.opendev.org/c/zuul/zuul-jobs/+/865459 | 16:28 |
| *** marios is now known as marios|out | 16:47 | |
| fungi | ianw: clarkb: the system-config-run-letsencrypt failure on https://review.opendev.org/865937 is baffling, what am i missing? | 17:00 |
| fungi | while the quote escaping shown by ansible looks questionable, the way that parameter is set and used seems to pre-date the change | 17:01 |
| clarkb | fungi: it looks like it is running the issue command which I thought we weren't supposed to do in CI | 17:03 |
| fungi | oh, perhaps it's the condition not matching there | 17:04 |
| clarkb | we might be setting the flag to talk to the staging server in that job though | 17:04 |
| fungi | seems like the letsencrypt_self_generate_tokens option would control that | 17:05 |
| clarkb | if we do that then I think we expect things to work, however it relies on talking to their staging service | 17:05 |
| clarkb | letsencrypt_self_generate_tokens: False is set on the job | 17:05 |
| clarkb | now does it set the flag for using the staging service | 17:06 |
| clarkb | playbooks/zuul/templates/group_vars/letsencrypt.yaml.j2:letsencrypt_use_staging: True | 17:07 |
| clarkb | I've not found anything adding thosenodes to that group yet | 17:09 |
| clarkb | playbooks/zuul/templates/gate-groups.yaml.j2 that file does it | 17:11 |
| *** dviroel|lunch is now known as dviroel | 17:12 | |
| clarkb | fungi: https://zuul.opendev.org/t/openstack/build/c84b753277f14c3fbc96b9ccf174bce4/log/letsencrypt01.opendev.org/acme.sh/acme.sh.log#4 | 17:12 |
| clarkb | I think it might be --debug2 | 17:12 |
| clarkb | not --debug=2 | 17:12 |
| fungi | ooh. so the quoting was a red herring | 17:14 |
| fungi | unfortunately no_log eating all the output from the command makes it hard to reason through from just the logs | 17:15 |
| fungi | https://github.com/acmesh-official/acme.sh/wiki/How-to-debug-acme.sh says "--debug 2" though | 17:16 |
| *** jpena is now known as jpena|off | 17:17 | |
| fungi | according to https://github.com/acmesh-official/acme.sh/wiki/Enable-acme.sh-log there's also a "--log-level 2" | 17:18 |
| clarkb | fungi: I'm not sure what quoting you are looking at | 17:23 |
| clarkb | I just followed the errors reported by zuul and it led to the log file complaining about the invalid option | 17:23 |
| fungi | oh, i keep forgetting about the redirected log being available. i was looking at the ara report for the failing nested ansible task, and going by how additional escaped quotes were wrapping the list of cli args being passed after the issue subcommand | 17:24 |
| fungi | i quoted it in my first review comment | 17:25 |
| fungi | https://0bf4aecb8c3e109f5411-ea455149ffcdba396338d4b9ec44d2c2.ssl.cf5.rackcdn.com/865937/2/check/system-config-run-letsencrypt/c84b753/bridge99.opendev.org/ara-report/results/491.html | 17:25 |
| fungi | the _raw_params string specifically | 17:25 |
| fungi | maybe the shell eventually ends up eating those additional quotes before tokenizing though | 17:26 |
| fungi | but yeah, https://zuul.opendev.org/t/openstack/build/c84b753277f14c3fbc96b9ccf174bce4/log/letsencrypt01.opendev.org/acme.sh/acme.sh.log#4 looks far more likely to be the cause of the nonzero exit, and makes sense as being the result of the proposed change | 17:28 |
| fungi | so maybe the option lexer in acme.sh doesn't handle = and requires space-separated option/value pairs instead? | 17:29 |
| fungi | i'll give that a try | 17:29 |
| clarkb | I think --debug2 may be special | 17:29 |
| clarkb | let me loo | 17:29 |
| clarkb | --debug [0|1|2|3] Output debug info. Defaults to 1 if argument is omitted. | 17:29 |
| clarkb | I guess it would be --debug 2 | 17:30 |
| opendevreview | Jeremy Stanley proposed opendev/system-config master: letsencrypt: add variable for more verbose debugging https://review.opendev.org/c/opendev/system-config/+/865937 | 17:31 |
| fungi | cool. in which case that ^ removes the = | 17:31 |
| fungi | and replaces it with a space | 17:31 |
| clarkb | any reason t not approve https://review.opendev.org/c/opendev/system-config/+/865930 ? | 17:33 |
| fungi | i wasn't sure if i had adequately answered frickler's question | 17:34 |
| clarkb | I'm going to go ahead and approve it so that we can check for happy certs tomorrow and also make progress on mailman3 | 17:37 |
| fungi | thanks! | 17:39 |
| fungi | i was hoping to go ahead and approve the remaining topic:mailman3 changes as soon as that merges | 17:40 |
| frickler | fungi: clarkb: I'm fine with 3.0.5 as workaround for now, I'm just concerned that it may be as difficult to move away from it again as with the grafana tag | 18:08 |
| clarkb | yup I'm hoping the upstream issue helps address that | 18:08 |
| opendevreview | Dr. Jens Harbott proposed zuul/zuul-jobs master: Use recent node version for markdownlint job https://review.opendev.org/c/zuul/zuul-jobs/+/866055 | 18:14 |
| opendevreview | Merged opendev/system-config master: letsencrypt: pin acme.sh to 3.0.5 https://review.opendev.org/c/opendev/system-config/+/865930 | 18:32 |
| opendevreview | Merged opendev/system-config master: Add lists01.opendev.org to the inventory https://review.opendev.org/c/opendev/system-config/+/865361 | 19:15 |
| opendevreview | Merged opendev/system-config master: bridge: switch OSC from container to local install https://review.opendev.org/c/opendev/system-config/+/865606 | 19:41 |
| fungi | oh, in addition to checking blocklists, i sent a test message from a shell on the new lists01 server to my personal address to check for any obvious red flags my spamassassin filter might throw | 19:56 |
| fungi | infra-pro-base failed for the production mm3 server deployment: https://zuul.opendev.org/t/openstack/build/59b05a7ff175438dbdb0e378f02fc930 | 20:16 |
| fungi | ansible log says: | 20:16 |
| fungi | fatal: [lists01.opendev.org]: UNREACHABLE! | 20:16 |
| fungi | Failed to connect to the host via ssh: Host key verification failed. | 20:16 |
| fungi | ianw: maybe we have a race condition with adding known_hosts entries and connecting to new servers? | 20:17 |
| fungi | there are lists01.opendev.org entries in /etc/ssh/ssh_known_hosts with a last modified time of 20:05 on the file | 20:18 |
| fungi | but looks like bridge probably tried to connect at 19:37 | 20:19 |
| fungi | i guess the next hourly will catch it up, but probably we need to make sure that the keys are plugged into bridge before we do any remote tasks? | 20:19 |
| fungi | infra-root: draft mm3 migration announcement for the opendev and zuul lists sites is available for review near the top of https://etherpad.opendev.org/p/mm3migration now (just after the migration plan steps). feel free to mark it up and i'll incorporate changes and send sometime tomorrow if there's no disagreements and assuming we're not still struggling to get the configuration rolled out to | 20:21 |
| fungi | the server | 20:21 |
| ianw | fungi: hrm, that should be done in the "bootstrap-bridge" step, which should run before any other job | 20:42 |
| ianw | hrm, which didn't trigger in the buildset -> https://zuul.opendev.org/t/openstack/buildset/0709aa73d4124175b0a21146f1cfc77e | 20:44 |
| ianw | i'll have to think about that | 20:44 |
| fungi | interesting. i guess it also happened in one of the other jobs which ran, just not early enough | 20:45 |
| fungi | so i suppose bootstrap-bridge needs to trigger any time a change alters inventory/base/hosts.yaml | 20:46 |
| opendevreview | Jeremy Stanley proposed opendev/system-config master: Also bootstrap bridge any time inventory changes https://review.opendev.org/c/opendev/system-config/+/866069 | 20:50 |
| fungi | ianw: so that ^ ? | 20:50 |
| ianw | yeah, for parallel running, it should basically always trigger, as that would be the point we synchronize the system-config checkout, before letting the other jobs run against it | 20:50 |
| fungi | oh, are you saying this is a transitional state and the parallel deploy work will address it anyway? | 20:51 |
| ianw | fungi: i think so -- https://review.opendev.org/c/opendev/system-config/+/820320/6/zuul.d/infra-prod.yaml would remove the matchers so the job always runs | 20:52 |
| ianw | but for the time being, i think your change is correct | 20:52 |
| fungi | i can adjust it to just do what 820320 does so there's no merge conflict | 20:53 |
| fungi | if you prefer | 20:53 |
| ianw | no that's fine, thanks, i think stick with the simple thing first | 20:53 |
| fungi | also it appears the osc change didn't successfully deploy either | 20:54 |
| ianw | refusing to convert from file to symlink for /usr/local/bin/openstack | 20:55 |
| ianw | i guess i should have rm'd that first. i think i'll just manually do that on the server | 20:55 |
| fungi | wfm | 20:56 |
| fungi | we're never going to need to upgrade any more bridge servers with that change anyway | 20:57 |
| fungi | i suppose if ansible's file module has something equivalent to ln's -f option, we could have used that (if we'd thought about it) | 20:58 |
| ianw | yeah i should probably have ensured absent: before hand. anyway i re-ran service-bridge and it's setup now | 20:59 |
| ianw | (it == openstack command) | 20:59 |
| fungi | thanks! | 21:00 |
| ianw | 3.5.4 does have the speed improvements for the copy-approvals included (Split the work into slices of changes and then utilize one thread per slice. Also skip unnecessary reindexing. On a large Gerrit site with 1.3 million changes, this reduced the time for copy-approvals from 5 hours to 15-20 minutes.) | 21:05 |
| ianw | so perhaps hours is over-estimating now | 21:06 |
| fungi | we're about 2/3 that size | 21:06 |
| fungi | what is copy-approvals? | 21:07 |
| Clark[m] | One thing about copy approvals is there aren't a ton of docs so it's hard to know what time expect like how do we know it is done etc | 21:07 |
| fungi | is that an upgrade time data migration? | 21:08 |
| Clark[m] | It can be run online so the suggestion is you run well in advance of your upgrade | 21:12 |
| Clark[m] | A number of people had failures with it and this latest 3.5 release fixes some of those too | 21:12 |
| Clark[m] | But yes required to run while on 3.5 before upgrading to 3.6 and can be online or offline | 21:12 |
| ianw | Clark[m]: afaics from looking at post 3.5.4 things in https://gerrit.googlesource.com/gerrit/+log/refs/heads/stable-3.5 -- i don't think there's any more additions to the copy-approvals bits | 21:13 |
| ianw | reading the threads, nothing specific i can find about 3.5.4 issues with copy-approvals | 21:17 |
| ianw | one thing people have been calling out is maybe running it individually for each project to make it clear if any fails | 21:17 |
| ianw | "Turns out the only one that failed was All-Projects... 🤔" -- https://groups.google.com/d/msgid/repo-discuss/CAALJ1-nM3K%2BgandnBOA4VSvYU5U2JuP490-xXDry6NS%2BsEATWA%40mail.gmail.com?utm_medium=email&utm_source=footer | 21:18 |
| ianw | my current thinking is that after school run i'll do a test on just one project. i'll see what sort of logs that produces and then we can have something that goes through each project in ls-projects one by one | 21:26 |
| ianw | i think that's a bit more conservative than letting it run globally in parallel and then having to pick apart the failures | 21:26 |
| ianw | we're not in any particular rush | 21:27 |
| Clark[m] | That sounds like a great plan | 21:38 |
| *** dasm is now known as dasm|off | 21:39 | |
| Clark[m] | There are a handful of projects/changes that have problems with reindexing today. One of those may be a good option? I'm trying to remember what they are now but it's due to some changes with incomplete data iirc | 21:42 |
| fungi | maybe at least run it separately for the all-projects repo | 21:45 |
| fungi | and all-users | 21:45 |
| Clark[m] | I would expect those to be pretty safe since we don't do reviews on them. What copy approvals does is copy votes from old patchsets to new patchsets when those votes carry over due to trivial rebases and such | 21:50 |
| Clark[m] | But can't hurt to do them that way either | 21:51 |
| fungi | oh, yes then i agree it's more likely we'll see issues with repos which have been around since the dawn of 2012 | 21:57 |
| fungi | since they may not test migrating reviews from gerrit 1.x | 21:57 |
| *** dviroel is now known as dviroel|out | 22:00 | |
| clarkb | fungi: ya I think the issues they already fixed were corner cases like that | 22:18 |
| ianw | so the output looks like -> https://paste.opendev.org/show/btjcjw7KfDgYxRV6wc2p/ | 22:26 |
| clarkb | ianw: ah ok so the output gives you progress in the foreground and we don't need to dig through the error log presumably. Thats good | 22:28 |
| ianw | looking at one of the modified changes, https://review.opendev.org/c/openstack/diskimage-builder/+/89531, doesn't seem to have any side-effects (e.g. like bumping it to the top of the review list) | 22:28 |
| clarkb | looks like December Tuesday largely dodge the major holidays? December 20, 27, and January 3. That said I don't really expect everyone to be around all those days so we may still skip some or all of them | 22:42 |
| clarkb | for myself I'll probably be around for December 20 and January 3. | 22:43 |
| fungi | yah i'll be around but happy to skip | 22:50 |
| clarkb | frickler: post /opt movement I'm seeing / 37GB total, 3.8GB used, 32GB available /opt is ~12GB used out of 74GB total. Pre /opt movement / is 37GB total, 14GB used, 22GB available. | 22:50 |
| clarkb | Partitioning and formatting isn't terribly slow so we could theoretically continue to make a swap partition and add the 74GB /opt and somehow mount the preexisting /opt over that | 22:51 |
| clarkb | can you bind mount in the same namespace with overlapping names? | 22:52 |
| ianw | ok, i have a small script running in a screen on review, output to ~ianw/copy-approvals | 22:53 |
| fungi | cow mount the extra fs over top of /opt maybe? | 22:53 |
| clarkb | fungi: is that doable? that might be the ticket | 22:53 |
| opendevreview | James E. Blair proposed ttygroup/gertty master: WIP: support inline comment threads https://review.opendev.org/c/ttygroup/gertty/+/860588 | 22:53 |
| clarkb | ianw: and that script is doing copy approvals for everything? | 22:53 |
| fungi | i totes made it up just now, but maybe? | 22:53 |
| clarkb | fungi: looking at mount's manpage for --bind I don't see anything allowing one to be more specific than a file path which complicates things if we don't want to break existing paths | 22:54 |
| ianw | clarkb: yep, it's looping through each project; ~ianw/copy-approvals/logs has the output for each | 22:54 |
| clarkb | ianw: I wonder if we should rerun it afterwards with no project filters just to be sure there aren't any hidden "projects" that gerrit will want torun it against too. That seems unlikely but gerrit is good at surprising me | 22:55 |
| ianw | sure, can do | 22:55 |
| ianw | so far everything seems to be working -- or at least the command is giving a return code of 0 | 22:55 |
| corvus | fungi: mm announcement email lgtm | 22:55 |
| ianw | hasn't hit openstack/nova yet though :) | 22:56 |
| clarkb | fungi: mailing list work announcement lgtm as well | 22:57 |
| fungi | thanks! | 22:57 |
| corvus | fungi: the latest update to 860588 should have significant improvements to the comment editing. i'd say the patch is about 80% complete now (the diff screen needs more work), but i believe at this point there should be no more disappearing comments. unified diff mode is lagging behind side-by-side a bit. | 22:59 |
| corvus | at this point, while i wouldn't recommend it for the casual user, it should work well enough without any hidden gotchas for the adventurous user. | 22:59 |
| corvus | clarkb: re the gerrit upgrade topic -- maybe something to put on the todo list for the time that we spend on 3.6 is to upgrade our project.config files to match the 3.7 expectations (even though it shouldn't break existing projects, it would be good to have the same configs for old + new) | 23:01 |
| clarkb | corvus: I think I parse what you mean but to be sure you're saying once we are on 3.6 we should take a moment to synchronize acls for 3.7's expectations? if so I agree | 23:02 |
| corvus | yep exactly | 23:03 |
| clarkb | fungi: not having openstacksdk listed in https://review.opendev.org/c/opendev/system-config/+/865320/5/launch/pyproject.toml means that pip won't upgrade it to latest because 0.102 will be sufficient I believe | 23:03 |
| clarkb | fungi: do we want to explicitly ask for >=0.103 or manually update it. I'm leaning towards being explicit as we've found this whole thing to be confusing over time | 23:03 |
| ianw | probably not a bad idea to drop a comment with a > just for context | 23:09 |
| ianw | ok, finished for all projects. none returned an error code, which is nice | 23:21 |
| clarkb | nice | 23:23 |
| ianw | $ cat * | grep updated | wc -l | 23:26 |
| ianw | 5580 | 23:26 |
| ianw | it doesn't really seem like that many changes were really updated | 23:27 |
| clarkb | its only the subset of changes that have votes that need to be forward ported to newer patchsets | 23:29 |
| clarkb | the old system apparently looked at all patchsets and figured out which votes apply to the current one but current Gerrit wants to reapply those votes as those events occur so it can view only the current patchset | 23:30 |
| clarkb | ianw: I left comments on https://review.opendev.org/q/topic:rax-rdns | 23:30 |
| clarkb | the first change in the tsakc has a bug otherwise its mostly me just talking out loud | 23:30 |
| ianw | ok thanks, will go over | 23:31 |
| clarkb | and I've approved the idnetity api filter removal for rax | 23:32 |
| corvus | clarkb: okay i dug into the gerrit 3.7.0 thing -- the latest is that it's accepted as a gerrit bug, nothing for zuul to fix. bug here: https://bugs.chromium.org/p/gerrit/issues/detail?id=16475 patch here: https://gerrit-review.googlesource.com/c/gerrit/+/353017 | 23:38 |
| corvus | clarkb: i think our action is to wait until that lands (or possibly we could do a test build if we want to double check that patch) | 23:38 |
| clarkb | I expect the original reporter may be able to test or we could reach out and offer to help them test? | 23:41 |
| corvus | luca requested they test | 23:41 |
| clarkb | ah perfect if a test is added in the fix change that is probably good enough for our needs | 23:41 |
| corvus | (but also, luca tested locally and plans to add a unit test, so yeah, i think that sounds good enough unless someone requests otherwise) | 23:42 |
| fungi | corvus: thanks! i'll refresh my install | 23:46 |
| corvus | amusingly, i had trouble reproducing at first because my local js client was 3.6.x, but after a shift-reload saw the issue. that may be something to keep in mind if any users report not seeing new features after upgrades :) | 23:47 |
| ianw | clarkb: good call on rerunning. when rerunning it seeemed to find another 24 changes to update (~ianw/copy-approvals/rerun.txt) | 23:47 |
| fungi | clarkb: ianw: that's a good point, we likely want to force newer sdk in order to make sure it actually upgrades, or completely recreate that venv on update | 23:48 |
| ianw | i might re-run it *again* to see if it has become idempotent | 23:49 |
| fungi | yeah, it could keep finding those same 24 every time now | 23:50 |
| clarkb | corvus: I think I may have identified a bug in luca's fix. I left comments to illustrate it | 23:50 |
| clarkb | ianw: ++ | 23:50 |
| clarkb | ianw: and maybe even rerun it again the day before we upgrade or something to make sure old data isn't being inserted somehow | 23:51 |
| *** rlandy is now known as rlandy|out | 23:51 | |
| corvus | clarkb: i'll repro your situation and see what stream-events emits | 23:51 |
| corvus | oh er that won't be useful without lucas patch, nevermind | 23:52 |
| clarkb | I only realized this because I looked at luca's comment and realized I could reply to it. | 23:53 |
| corvus | clarkb: fwiw, i believe that matches the existing behavior -- here's davido's original fix from a few years ago: https://gerrit-review.googlesource.com/c/gerrit/+/291144/16/java/com/google/gerrit/server/restapi/change/PostReview.java | 23:53 |
| corvus | so i agree with you that it's potentially weird, but also, it's probably not a regression from the current state | 23:54 |
| clarkb | neaty | 23:55 |
| clarkb | *neat | 23:55 |
| corvus | a potential way to disambiguate would be to look for the patchset level comment with no in_reply_to set | 23:55 |
| corvus | (now you know why i batched this work with the gertty work -- i paged all that in at the same time) | 23:55 |
| clarkb | you're probably the expert on this now too :P | 23:56 |
| * corvus hides | 23:56 | |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!