| amotoki | vishalmanchanda: could you check my comment in https://review.opendev.org/c/openstack/horizon/+/806454 ? | 04:26 |
|---|---|---|
| amotoki | vishalmanchanda: I see several regressions after this commit was merged. | 04:26 |
| vishalmanchanda | amotoki: yeah looking. | 04:27 |
| amotoki | vishalmanchanda: I am not sure we should revert it first or wait for the fix? | 04:27 |
| vishalmanchanda | amotoki: but did you try if breaks anything in UI side. | 04:28 |
| vishalmanchanda | amotoki: I mean I have checked the UI for project/routers/ports/forms.py#L50 and project/network_topology/views.py#L265-L268 | 04:29 |
| vishalmanchanda | amotoki: it works same as before | 04:29 |
| vishalmanchanda | amotoki: Let's wait for ganso reply on your comments. | 04:29 |
| amotoki | vishalmanchanda: ah, I got it. I misunderstood it as I tried to touch the code and it fails. | 04:38 |
| amotoki | vishalmanchanda: _perform_query changes the type of return value dependin on parameters..... | 04:39 |
| amotoki | vishalmanchanda: super tricjy. it is not a good idea to change the return type as such from the software engineering perspectice. | 04:39 |
| amotoki | vishalmanchanda: at least I cannot say I can maitnaain api/neutron.py...... :-( | 04:40 |
| vishalmanchanda | amotoki: yeah too many tricks in the patch | 04:40 |
| amotoki | vishalmanchanda: so what is your plan to make it easier to maintain? | 04:41 |
| vishalmanchanda | amotoki: but I +2ed it because pagination for these pending from long time and encourage new contributor. | 04:41 |
| vishalmanchanda | amotoki: I was thinking if can define a common mechanism for pagination which all backend services can use | 04:42 |
| amotoki | vishalmanchanda: I am not against merging it, but if there is such trick which may lead to big maintenance debt I believe at least you need to discuss how to address the debt soon. | 04:42 |
| amotoki | vishalmanchanda: no, I don't think it can be addressed by adopting the common mechanism for pagination. | 04:43 |
| amotoki | vishalmanchanda: it is a function definition of network_list_for_tenant. | 04:43 |
| amotoki | vishalmanchanda: it changes the type of the return value conditionally | 04:43 |
| amotoki | vishalmanchanda: if such, IMO we should have two separate functions. | 04:44 |
| vishalmanchanda | amotoki: ohh I see, will bring it next weekly meeting. | 04:45 |
| amotoki | at least I am against having two types of return value from a single function. Perhaps you can remember all :) | 04:47 |
| -amotoki- is joking :) I usually forgot what I wrote one month before :p | 04:48 | |
| vishalmanchanda | amotoki: lol:) | 04:48 |
| vishalmanchanda | amotoki: Also, like to know your opinion about https://review.opendev.org/c/openstack/horizon/+/817570 | 04:50 |
| vishalmanchanda | amotoki: It's a small bp and looks good to me only few nits thing needs to improved. | 04:51 |
| amotoki | vishalmanchanda: I am not sure I can have enough time to review it this week. | 04:51 |
| vishalmanchanda | amotoki: ok np. | 04:51 |
| *** prometheanfire is now known as Guest2 | 04:59 | |
| *** Guest2 is now known as prometheanfire | 05:09 | |
| amotoki | vishalmanchanda: how the feature freeze works in horizon? is it applies same as for cycle-with-rc projects? | 05:27 |
| vishalmanchanda | vishalmanchanda: yes. | 05:27 |
| amotoki | vishalmanchanda: I mean "this week" above. when I wrote it I thought there is only Tue and Wed to land it, so I cannot promise it. | 05:27 |
| vishalmanchanda | amotoki: ok. | 05:28 |
| amotoki | vishalmanchanda: I can drop review comments now, but I am not sure I can have enough loops. | 05:28 |
| amotoki | vishalmanchanda: as a quick look, there are several things I would like to comment but they might be minor | 05:29 |
| vishalmanchanda | amotoki: thanks:) | 05:29 |
| -amotoki- hopes he no longer maintains api/neutron.py. | 07:17 | |
| amotoki | network_list() was the top level interface but it now seems for both public and private functions..... :-( | 07:17 |
| amotoki | I tried to refactor it but gave it up as of today. hopefullly someone can maitnain the code. | 07:18 |
| amotoki | this is the main reason I could not review it well. too many comments forced me to give up commeting :-( | 07:19 |
| opendevreview | Thomas Goirand proposed openstack/horizon stable/xena: Fix for "Resize instance" button https://review.opendev.org/c/openstack/horizon/+/829662 | 13:57 |
| opendevreview | Vadym Markov proposed openstack/horizon master: Add parameter validation for namespaces request https://review.opendev.org/c/openstack/horizon/+/829901 | 14:12 |
| amotoki | zigo: as I replied in the backport review, the backport itself is required. I can check it tomorrow. (it is late for me ;p) thanks for the backport! | 16:08 |
| opendevreview | Merged openstack/horizon master: Add Rules operation to Network QoS Policy https://review.opendev.org/c/openstack/horizon/+/805360 | 20:20 |
Generated by irclog2html.py 2.17.3 by Marius Gedminas - find it at https://mg.pov.lt/irclog2html/!