Hi, I have PBI with idea that now when we have auto wrapping of lines for Label widget we could use it for Popup module. Reason is that it is often constructed and sometimes it include even output of some command. And if output has too long line, it is not visible. So idea looks good and when I use it for new Yast2::Popup class it works fine[1]. But when I try it for old Yast::Popup it is broken[2]. After debugging it appears it is caused by Stretch[3]. So my question now is how should I proceed? Possible options (do not hesitate to write other if I overlook something): 1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API. 2. Remove stretches from Yast::Popup. It can possible break layout somewhere and probably openQA would need to be adopted. 3. similar to 2. just instead of adapting layout, use from Yast::Popup calls to Yast2::Popup and keep it just as legacy backward compatible layer. As there is a plan to do some changes that affects openqa in January, I think that deadline to the end of December is good enough. My preferred solution is 1 as it basically start converging to one API. Josef [1] pr with screenshot: https://github.com/yast/yast-yast2/pull/1122 [2] pr with changes and screenshot how much it is broken in common installation: https://github.com/yast/yast-yast2/pull/1121 [3] pr with screenshot and minimal example for which I can reproduce it: https://github.com/yast/yast-ycp-ui-bindings/pull/64
On Wed, 2020-12-09 at 16:56 +0100, josef Reidinger wrote:
Hi, I have PBI with idea that now when we have auto wrapping of lines for Label widget we could use it for Popup module. Reason is that it is often constructed and sometimes it include even output of some command. And if output has too long line, it is not visible. So idea looks good and when I use it for new Yast2::Popup class it works fine[1]. But when I try it for old Yast::Popup it is broken[2]. After debugging it appears it is caused by Stretch[3]. So my question now is how should I proceed? Possible options (do not hesitate to write other if I overlook something):
1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API.
2. Remove stretches from Yast::Popup. It can possible break layout somewhere and probably openQA would need to be adopted.
3. similar to 2. just instead of adapting layout, use from Yast::Popup calls to Yast2::Popup and keep it just as legacy backward compatible layer.
As there is a plan to do some changes that affects openqa in January, I think that deadline to the end of December is good enough. My preferred solution is 1 as it basically start converging to one API.
I prefer the first option too. Regards.
Josef
[1] pr with screenshot: https://github.com/yast/yast-yast2/pull/1122 [2] pr with changes and screenshot how much it is broken in common installation: https://github.com/yast/yast-yast2/pull/1121 [3] pr with screenshot and minimal example for which I can reproduce it: https://github.com/yast/yast-ycp-ui-bindings/pull/64 _______________________________________________ YaST Development mailing list -- yast-devel@lists.opensuse.org To unsubscribe, email yast-devel-leave@lists.opensuse.org List Netiquette: https://en.opensuse.org/openSUSE:Mailing_list_netiquette List Archives: https://lists.opensuse.org/archives/list/yast-devel@lists.opensuse.org
On 12/9/20 4:56 PM, josef Reidinger wrote:
1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API.
2. Remove stretches from Yast::Popup. It can possible break layout somewhere and probably openQA would need to be adopted.
3. similar to 2. just instead of adapting layout, use from Yast::Popup calls to Yast2::Popup and keep it just as legacy backward compatible layer.
As there is a plan to do some changes that affects openqa in January, I think that deadline to the end of December is good enough. My preferred solution is 1 as it basically start converging to one API.
I would probably prefer #3 as... - We expect openQA changes anyway - Deprecated classes usually live forever anyway - #1 can start and never finish JM2cents Lukas -- Lukas Ocilka, Systems Mgmt & Consulting Team Leader & YaST Product Owner SLE Department, SUSE Linux I don't know everything I don't know
On Thu, 10 Dec 2020 10:30:53 +0100
Lukas Ocilka
On 12/9/20 4:56 PM, josef Reidinger wrote:
1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API.
2. Remove stretches from Yast::Popup. It can possible break layout somewhere and probably openQA would need to be adopted.
3. similar to 2. just instead of adapting layout, use from Yast::Popup calls to Yast2::Popup and keep it just as legacy backward compatible layer.
As there is a plan to do some changes that affects openqa in January, I think that deadline to the end of December is good enough. My preferred solution is 1 as it basically start converging to one API.
I would probably prefer #3 as...
- We expect openQA changes anyway - Deprecated classes usually live forever anyway - #1 can start and never finish
Thanks for answer. Well, that is nice about 1. It is not intended to be finished. It is just deprecated/obsolete API that is kept as it works, but if any problem appear, the solution is to use new API. So maybe as time goes its usage will lower and lower and maybe in the end it will be so small, that we drop it. But goal of #1 is not removing anything just do not touch that old code anymore.
JM2cents Lukas
Josef
On 12/9/20 3:56 PM, josef Reidinger wrote:
Hi, I have PBI with idea that now when we have auto wrapping of lines for Label widget we could use it for Popup module. Reason is that it is often constructed and sometimes it include even output of some command. And if output has too long line, it is not visible. So idea looks good and when I use it for new Yast2::Popup class it works fine[1]. But when I try it for old Yast::Popup it is broken[2]. After debugging it appears it is caused by Stretch[3]. So my question now is how should I proceed? Possible options (do not hesitate to write other if I overlook something):
1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API.
+1 to this option.
2. Remove stretches from Yast::Popup. It can possible break layout somewhere and probably openQA would need to be adopted.
3. similar to 2. just instead of adapting layout, use from Yast::Popup calls to Yast2::Popup and keep it just as legacy backward compatible layer.
As there is a plan to do some changes that affects openqa in January, I think that deadline to the end of December is good enough. My preferred solution is 1 as it basically start converging to one API.
Josef
[1] pr with screenshot: https://github.com/yast/yast-yast2/pull/1122 [2] pr with changes and screenshot how much it is broken in common installation: https://github.com/yast/yast-yast2/pull/1121 [3] pr with screenshot and minimal example for which I can reproduce it: https://github.com/yast/yast-ycp-ui-bindings/pull/64 _______________________________________________ YaST Development mailing list -- yast-devel@lists.opensuse.org To unsubscribe, email yast-devel-leave@lists.opensuse.org List Netiquette: https://en.opensuse.org/openSUSE:Mailing_list_netiquette List Archives: https://lists.opensuse.org/archives/list/yast-devel@lists.opensuse.org
-- Knut Alejandro Anderssen González YaST Team at SUSE Linux GmbH
Dne 09. 12. 20 v 16:56 josef Reidinger napsal(a):
1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API.
I'd prefer this solution as it is the least intrusive and can be done in small steps. I think the rule should be: if you are touching code which uses the old Yast::Popup then rewrite it to the new Yast2::Popup. And after some time we should fix the remaining old calls. We already used a similar approach elsewhere (removing the old testsuite), I think for the popups it should work well too. -- Ladislav Slezák YaST Developer SUSE LINUX, s.r.o. Corso IIa Křižíkova 148/34 18600 Praha 8
On 12/9/20 3:56 PM, josef Reidinger wrote:
Hi, I have PBI with idea that now when we have auto wrapping of lines for Label widget we could use it for Popup module. Reason is that it is often constructed and sometimes it include even output of some command. And if output has too long line, it is not visible. So idea looks good and when I use it for new Yast2::Popup class it works fine[1]. But when I try it for old Yast::Popup it is broken[2]. After debugging it appears it is caused by Stretch[3]. So my question now is how should I proceed? Possible options (do not hesitate to write other if I overlook something):
1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API.
2. Remove stretches from Yast::Popup. It can possible break layout somewhere and probably openQA would need to be adopted.
3. similar to 2. just instead of adapting layout, use from Yast::Popup calls to Yast2::Popup and keep it just as legacy backward compatible layer.
As there is a plan to do some changes that affects openqa in January, I think that deadline to the end of December is good enough. My preferred solution is 1 as it basically start converging to one API.
Josef
I also prefer the option 1. Even, I would print a warning when the deprecated Yast::Popup is used (similar to what ruby does when some API is deprecated). But, on the other hand, personally I don't like how we usually deprecate APIs in YaST. Basically, we add new classes but we never remove the deprecated ones. As result, we accumulate a lot of zombies. Maybe, this time we could go one step forward? Regards, Iván -- José Iván López González YaST Team at SUSE LINUX GmbH IRC: jilopez
On Fri, 11 Dec 2020 09:18:46 +0000
José Iván López González
On 12/9/20 3:56 PM, josef Reidinger wrote:
Hi, I have PBI with idea that now when we have auto wrapping of lines for Label widget we could use it for Popup module. Reason is that it is often constructed and sometimes it include even output of some command. And if output has too long line, it is not visible. So idea looks good and when I use it for new Yast2::Popup class it works fine[1]. But when I try it for old Yast::Popup it is broken[2]. After debugging it appears it is caused by Stretch[3]. So my question now is how should I proceed? Possible options (do not hesitate to write other if I overlook something):
1. have auto wrapping just for Yast2::Popup and has Yast::Popup just as deprecated legacy class. And if bug report appear about long unreadable line, just use newer API.
2. Remove stretches from Yast::Popup. It can possible break layout somewhere and probably openQA would need to be adopted.
3. similar to 2. just instead of adapting layout, use from Yast::Popup calls to Yast2::Popup and keep it just as legacy backward compatible layer.
As there is a plan to do some changes that affects openqa in January, I think that deadline to the end of December is good enough. My preferred solution is 1 as it basically start converging to one API.
Josef
I also prefer the option 1. Even, I would print a warning when the deprecated Yast::Popup is used (similar to what ruby does when some API is deprecated).
But, on the other hand, personally I don't like how we usually deprecate APIs in YaST. Basically, we add new classes but we never remove the deprecated ones. As result, we accumulate a lot of zombies. Maybe, this time we could go one step forward?
Regards,
Iván
Well, my rough ( it includes also new Popup API) and blink grep shows quite impressive number: jreidinger@localhost:~/prace/yast> ack -ch Popup\. 28966 So really non-trivial work. Josef
participants (6)
-
David Díaz -
josef Reidinger -
José Iván López González -
Knut Alejandro Anderssen González -
Ladislav Slezak -
Lukas Ocilka