opendevreview | Merged openstack/ci-log-processing master: Try to always download build log files https://review.opendev.org/c/openstack/ci-log-processing/+/876691 | 10:20 |
---|---|---|
opendevreview | daniel.pawlik proposed openstack/ci-log-processing master: Remove build dirs with missing files; add more debug logs https://review.opendev.org/c/openstack/ci-log-processing/+/882270 | 11:41 |
*** elvira2 is now known as elvira | 14:01 | |
opendevreview | Vladimir Kozhukalov proposed openstack/project-config master: Remove retied projects from openstack-helm group https://review.opendev.org/c/openstack/project-config/+/884718 | 15:41 |
kozhukalov_ | Hi, I a PTL of Openstack-helm. I trying to cleanup the Storyboard and we have 3 retired projects. All the stories in these retired projects are closed and moved to the main project in the project group. As per agreement with the infra team I prepared this PR https://review.opendev.org/c/openstack/project-config/+/884718 which removes these retired projects from the project group and then someone with admin rights in SB | 15:58 |
kozhukalov_ | will change the description of these retired projects and will set them inactive. Can you please review the PR? | 15:58 |
fungi | kozhukalov_: yes, i had already pulled it up to look at, though i have a very full day of meetings so may not get to it right away | 15:59 |
kozhukalov_ | Thank you. It is not urgent at all. | 16:04 |
frickler | kozhukalov_: if you intend to stay around on IRC, you may also want to update your nick in the governance repo | 16:42 |
*** kozhukalov_ is now known as kozhukalov | 17:32 | |
kozhukalov | frickler: yes, thanks for this reminder. I prepared PR https://review.opendev.org/c/openstack/governance/+/884757 | 17:35 |
fungi | https://review.openstack.org/185785 "Alter governance repo voting rules" is what added the overridden code-review label to the openstack/governance acl | 18:31 |
fungi | the commit message includes, "Make Code-Review votes not required for a change to merge. This means they are purely advisory." | 18:32 |
clarkb | fungi: ya you can see that is preserved with the function NoBlock and submit requirement of is:true | 18:32 |
clarkb | makes it hard to say if those values should be copied or not since the category is mostly meaningless | 18:32 |
opendevreview | Merged openstack/project-config master: Remove retied projects from openstack-helm group https://review.opendev.org/c/openstack/project-config/+/884718 | 18:35 |
clarkb | gmann: yes the gerrit migrations from version 3.6 to 3.7 added those rules because copyAllScoresIfNoCodeChange was the default rule and this ruleset is supposed to be equivalent to that | 18:38 |
clarkb | gmann: if we didn't explicitly add the rules the ngerrit would add them and then the next time we pushed we would be out of sync and potentially it would reject the push | 18:38 |
clarkb | that is why it was added. It is possible there is a bug in Gerrit's assertion they are equivalent | 18:38 |
clarkb | either way you can just set it to whatever value you prefer instead | 18:39 |
fungi | "NO_CODE_CHANGE" allows the commit message to be different | 18:39 |
fungi | so that's at least why the score is getting copied | 18:39 |
gmann | but if copyAllScoresIfNoCodeChange was true by default then why we did not see this behavior early | 18:39 |
clarkb | gmann: as I said ther may be a bug in gerrit's assertion these are equivalent | 18:39 |
clarkb | we were just operating with what gerrit would forcibly do for us and capturing that in our out of band config to avoid issues with configuration amangement | 18:39 |
gmann | yeah, I remember I did many commit msg only change in governance many time | 18:39 |
gmann | times | 18:39 |
clarkb | basically if you had no function set then it would add these rules. If we didn't add these rules to our config they would be added behind the scenes. Then when you tried to make a change in project-config it would fail to push due to missing necessary rules | 18:40 |
clarkb | so we got project-config in sync with what gerrit would do | 18:40 |
clarkb | to avoid that problem | 18:40 |
fungi | https://review.opendev.org/880115 indicates it was added to any labels which didn't previously specify "copyAllScoresIfNoChange = false" | 18:43 |
clarkb | fungi: ya because the default is "copyAllScoresIfNoChange = true" | 18:44 |
gmann | yeah, as default is true | 18:44 |
clarkb | fungi: and beacuse of that default gerrit woud autmatoically add these new rules in the upgrade migrations | 18:44 |
clarkb | in order to prevent that causing problems we did the work upfront so that it was tracked and managed | 18:44 |
fungi | right, i'm just trying to think through what would have previously been preventing those scores from being copied prior to the upgrade | 18:46 |
gmann | can we check if that was true by default because I see many repo set those as true explicitly. may be default value was changed | 18:48 |
gmann | recently | 18:48 |
clarkb | you can look at the gerrit docs, but ya I mean I think if you're asserting that wasn't the behavior it isn't going to cahnge anything | 18:49 |
clarkb | which is I think my point. Let's stop worrying about what may have been the behavior and just move forard. We had to address thigns for the gerrit upgrade and now y oucan just set the config to whatever you want | 18:50 |
fungi | agreed | 18:52 |
opendevreview | Jay Faulkner proposed openstack/pbr master: Replace imp with importlib.machinery https://review.opendev.org/c/openstack/pbr/+/884789 | 22:27 |
JayF | ^ is imported as I saw someone proposing a downstream gentoo patch to fix this, and they PR'd it to github, so I thought I'd ferry it over :-) I'll take a look at tests if anything blows up | 22:34 |
clarkb | JayF: ya I'm reviewing it quickly. It isn't python2 safe | 22:37 |
JayF | I wasn't sure if that was done on a branch, I figured unit tests would get angry if it was unsafe | 22:38 |
JayF | just need to guard the import like usual for python version stuff? | 22:38 |
JayF | try import; except Importerror import X | 22:38 |
clarkb | ya I'm writing a fix | 22:38 |
JayF | ack thank you | 22:38 |
clarkb | and will put it in the comments | 22:38 |
clarkb | because we don't test under pypy but pypy is used for a lot of python 2.7 I think this is an issue but also hard to test... | 22:41 |
JayF | I figured this was one of those "it's a two line fix except the ten lines that prevent those two lines from breaking something else" | 22:42 |
clarkb | for the unittest failurs i think we have to remove build_sphinx testing since that isn't a thing setuptools is doing anymore | 22:46 |
JayF | clarkb: pbr/tests/test_packaging.py:71:5: E731 do not assign a lambda expression, use a def | 22:47 |
clarkb | and the doc build errors appears related to setup_command imports? | 22:47 |
JayF | clarkb: I assume noqa this? | 22:47 |
JayF | I was hoping it wasn't unit test deterioration but just it being broken on my system :/ | 22:48 |
clarkb | JayF: I probably would? the idea is to keep the function definition small and in the import exception. But you could define a normal function there instead | 22:48 |
clarkb | oh setup_command is something we were importing form sphinx which is gone now | 22:49 |
clarkb | I think because setuptools has killed build_sphonx | 22:49 |
clarkb | so that all probably needs to be ripped out as killed by upstream integration ponts | 22:49 |
clarkb | cc fungi | 22:49 |
opendevreview | Jay Faulkner proposed openstack/pbr master: Replace imp with importlib.machinery https://review.opendev.org/c/openstack/pbr/+/884789 | 22:53 |
JayF | that ^ does not have all the unit test/docs fixes, but does implement a better fix, I believe | 22:53 |
JayF | I need to wrap up something before my EOD, I'll check that patch for further comments and help get it in | 22:53 |
clarkb | ya we probably want the sphinx command cleanup in a separate change anyway | 22:54 |
clarkb | pbr/packaging.py alrady has a try except import on builddoc | 22:56 |
clarkb | we can probably leave that as is but then prevent docs from trying to autodoc and also have tests only cover it if testable? | 22:56 |
clarkb | or we delete it all... | 22:56 |
JayF | like, autodoc is still a thing in sphinx, and it works | 22:56 |
JayF | at least in IPA it still works | 22:56 |
* JayF suspects he's missing a detail of what that actually does | 22:57 | |
clarkb | JayF: ya sphinx and autodoc are fine. The problem is trying to drive them through setup.py appears to no longer be a thing | 22:59 |
clarkb | so basically we need to stop supporting the functionality in pbr and force people to run the commands directly rather than with setup.py | 23:00 |
clarkb | what makes it tricky is this is a breaking change, but should we break everyone or just those with new setuptools and setup.py | 23:00 |
opendevreview | Clark Boylan proposed openstack/pbr master: Remove sphinx doc building integration https://review.opendev.org/c/openstack/pbr/+/884791 | 23:03 |
clarkb | something like that maybe | 23:03 |
clarkb | and left a comment on what it might look like if we keep support. But like you my day is ending and I need to not get sucked into this right now :) | 23:05 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!