[...] >>> >>> The change was introduced exactly a year ago[3], do it means that >>> the >>> yast2-ftp-server is no longer relevant? >> >> Well, I use that module sometimes, but usually to just setup quickly >> ftp server, so not so often. >> >>> >>> - Do we have this problem in more places? I.e. Yast::FileUtils taking >>> the place of Object::FileUtils. >> >> Yes, we had it in past, e.g. String versus Yast::String is also >> problem and some others. My common recommendation is to always use >> `::` prefix when you want to use ruby functionality and avoid >> collision with YaST namespace. >> Other long term solution on which we are working is to have code >> under 'src/lib` that does not live in YaST namespace, so for YaST >> specific calls like Yast::FileUtils you have to use full path. >> >>> >>> >>> I don't have the answer for any of them ;) but I have a proposal[4] for >>> the last one: to implement the `method_missing` in the Yast::FileUtils >>> and fallback to Object::FileUtils. That way, we could avoid similar >>> crashes. >> >> Well, it helps in this one specific case. I agree it is a bit robust, >> but can lead to another bugs like If you want to use FileUtils.Exist >> and use instead FileUtils.exists? which is very similar but later >> does not respect changed SCR target. and some other methods that just >> differs in case ( which was already bad for old YCP code which has >> various mixtures ). Also it does not help for other modules. >> Also I am not so big fan of this magic as it can often lead to WTF >> when you read code and wonder how it can work when you see that it >> calls wrong object. But I am also not strong against as I see some >> value in it. >> >> Josef >> >>> >>> What do you think? >>> >>> [1] >>> https://ruby-doc.org/stdlib-2.6.5/libdoc/fileutils/rdoc/FileUtils.html >>> [2] >>> https://github.com/yast/yast-yast2/blob/master/library/general/src/modules/FileUtils.rb >>> >>> [3] https://github.com/yast/yast-ftp-server/pull/54 >>> [4] https://github.com/yast/yast-yast2/pull/997 >>> >> > > I would also avoid to use meta-programming magic like that. Sooner > than later it will bite your ass ;). Moreover (and even though it is > not published in any official place yet) we have some namespace > conventions for the new code [1], for example Y2Storage, > Y2ServicesManager, Y2Firewall, Y2Network, etc. So the new code will > not live under Yast namespace anymore and it will not be directly > affected by this problem. > > For older code, I would follow the Josef's suggestion: to use the > scope resolution operator to always start from the global scope (e.g., > ::FileUtils). > > [1] https://gist.github.com/imobachgs/d90940315bd0471ee00b2a68ba22fbe5 > Thank you for the feedback guys!
Sure, for a known and specific case, using the scope resolution operator `::` is the direct and expected solution. In fact, it is what I proposed initially in the internal Trello card created to address the bug. I simply wanted to avoid future crashes in the old code with a "cheaper" solution. Nevertheless, I completely understand the against given argues. The good thing is that, apart from your thoughts and useful information, you also give me (us) an interesting link. Hands-down, we should polish (if needed) and publish it in the YaST Documentation[1] site. Regards. [1] https://yastgithubio.readthedocs.io/en/latest/README/ -- David Díaz González YaST Team at SUSE Linux GmbH
signature.asc
Description: OpenPGP digital signature
