February 2016 - YaST Development - openSUSE Mailing Lists

[yast-devel] Refactoring madness: chapter two
by Ancor Gonzalez Sosa 24 Feb '16

24 Feb '16

This is a follow-up on my highly opinionated mail about software design. Be warned, it's much longer than chapter one. Sorry for that. This mail contains concrete examples from the recent yast2-storage rewrite in order to touch some solid ground (simplistic improvised examples didn't work very well in chapter 1). Please, please, PLEASE, don't take it as an attack to the old code or their writers. Once again, this is just as an explained dump of my very own mental checklist of things I care when writing or reviewing code. Meant as a starting point for discussion... but not via mailing list, since we want to get to a conclusion at some point. :-) 1) Responsibility distribution We all agree this is a key aspect of OOP design. But we seem to usually disagree on the result. Single responsibility principle is a nice guideline here... when not taken too far. Going for examples, the original SpaceMaker had the obvious responsibility of resizing and deleting partitions to make space, but also of holding the list of free slots (something I moved to Devicegraph) and of deciding/knowing what was the target size (:desired vs :minimum) for the following steps (something I moved one level up to the Proposal class). So the resulting SpaceMaker was more dumb... while other classes became... well, less dumb. We also had a separate DiskAnalyzer class that other classes queried to know stuff like what's the list of existing windows partitions, which media is being used for installation (we don't want to install on top of ourselves) and that kind of stuff. I "hided" the DiskAnalyzer class behind Devicegraph. So now other classes query those questions directly to the devicegraph object. After all those changes, somebody could argue that Devicegraph responsibility became to broad, against the single responsibility principle. For me it makes sense that devicegraph knows its own list of free slots or which one of its own disks are being used for installation. Simply because it makes sense(TM) and I "feel" it's its responsibility. I want my API to hide the fact that some of those calculations are delegated to a DiskAnalyzer class. The main advantage is that the "users" of Devicegraph don't have to know several other classes just to ask things about the system layout... that leads me to the next point. 2) Keep the dependencies between your classes to a minimum. Another topic in which one could thing that we are going to agree easily... but was not the case. The best thing I can do in that regard is just recommending the chapter titled "Managing dependencies" from the Sandi Metz book[1]. She simply put into words my view about the topic. Including a fairly obvious but still useful checklist on when an object has a dependency. That happens when the object knows: - The name of another class - The name of a message that it intends to send to someone other than self. - The arguments that a message requires and/or the order of those arguments Everytime I see that in a class, my dependencies alarm rings and I start asking myself: Does it make sense? How can avoid that? I really try hard to fight every dependency until I accept it must be there. And lately I find myself using dependency injection more and more. 3) Don't let singleton objects pollute your APIs In some situations, the singleton pattern is a necessary evil. But please restrict the evilness as much as possible. The original code had StorageManager.instance.probed (or similar stuff) in too many places. Several classes (DiskAnalyzer, PartitionCreator, SpaceMaker and StorageProposal) knew were to find the probed devicegraph by the holy grace of Singleton. The modified code hides that singleton behind the Devicegraph.probed class method and reads the probed devicegraph only once. Devicegraph is the only class that knows there is a singleton StorageManager. The devicegraph returned by it is then simply passed as argument to any other class needing it. Thus, DiskAnalyzer (and others) doesn't know anymore about the existence of StorageManager.instance, it just receive a devicegraph to analyze (the fact that this devicegraph is always the probed one is not DiskAnalyzer's business). The example in (4) has also to do with singleton objects. 4) Messages-driven design I can also become quite picky about who sends messages to who (like I did in [2]). In fact, that is an example of two thing I consider wrong. One is sending the messages to the wrong receiver. Another one is pollution caused by singleton. You send messages to an instance of FakeProbing to produce an effect on StorageManager.instance.probed, but there is no reference to StorageManager during that interaction. It just works because FakeProbing and the caller both know about StorageManager being singleton. Talking about messages, emitters, receivers and and all that can look too philosophical. But for me is the key factor when defining an API (I even draw UML interaction diagrams sometimes!). And for me, ending up with a sane API is the key reason for having code reviews. 5) A minor one: don't abuse well known names That's something I do all the time, but I'm trying to improve that. It really hurts the reviewer mind when something is called "factory", "singleton", "strategy", "presenter" and so on but doesn't follow the corresponding Factory, Strategy, WhatsNot patterns. To stop doing that when writing code is one of my new year's resolutions (because I suffer it when reading other's people code). 6) Single responsibility principle... again In (1) I said I can be considered slightly relaxed when applying the single responsibility principle to objects, at least to a purist eyes. I must clarify that I really observe that principle when writing methods within a class, so I end up with a lot of small methods that rely on each other. Not sure if that makes my code easier or more difficult to follow. It depends on the reader, I guess. Well, I think this is enough for a second mail. I guess I have already covered my most important reasons to "slow down" the delivery of already working code in the yast2-storage rewrite. Cheers. [1] http://www.sandimetz.com/products [2] https://github.com/yast/yast-storage/pull/193#discussion_r53643452 -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

2 2

Re: [yast-devel] yast2-rpm-handler
by Josef Reidinger 24 Feb '16

24 Feb '16

On Tue, 23 Feb 2016 19:16:35 +0200 "opensuse.lietuviu.kalba" <opensuse.lietuviu.kalba(a)gmail.com> wrote: > > 2016.02.23 14:27, Josef Reidinger rašė: > > Hi, I had vacation and being outside of internat waters, I will > > recheck it now. We need to find good solution for it as other > > packages maybe also need it and we should now place such workaround > > everywhere. Josef > > As /workaround everywhere/, I suggest to > 1) submit "gksu" package (as it contains `xsu`) from obs:GNOME > repository to default repositories; > 2) replace all `xdg-su -c ...` expressions into `xsu ...` . > Problem with gksu is that it require gtk...we used in past kdesu which was in kde-runtime and have similar problem with qt. As want both, it is a bit problematic. It is reason why we used to xdg-su. For me the best solution is to allow xdg-su somehow pass command as argument and not via stupid -c parameter. But it is probably long effort to do such change. I will try to write xdg-su upstream about it. Josef -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

1 0

[yast-devel] GitHub and Git tips
by Ladislav Slezak 24 Feb '16

24 Feb '16

I found this interesting document [1], it's a nice collection of GitHub and Git tips and tricks. Additionally there are links to some recorded presentations about the topic at the top of the document, worth of seeing as well... HTH [1] http://git.io/sheet -- Ladislav Slezák Appliance department / YaST Developer Lihovarská 1060/12 190 00 Prague 9 / Czech Republic tel: +420 284 028 960 lslezak(a)suse.com SUSE -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

1 0

[yast-devel] Pending Pull Requests
by yast-ci＠opensuse.org 24 Feb '16

24 Feb '16

This email is automatic generated from yast CI node. It lists of pull requests that have no activity more then three working days. If your module is listed, please check all pull request, why they are not merged yet. Pending requests in repository yast-autoinstallation: - Fix install rules SLE 12 GA (26 days) https://github.com/yast/yast-autoinstallation/pull/190 - [Review] Request from 'schubi2' @ 'yast/yast-autoinstallation/review_160113_bug_955878_merged_with_sp1' (40 days) https://github.com/yast/yast-autoinstallation/pull/179 Pending requests in repository yast-bootloader: - [do not merge] partial usage of config files gem (19 days) https://github.com/yast/yast-bootloader/pull/280 Pending requests in repository yast-country: - Some changes in the tumbleweed mappings (bsc#942899 and bsc#942896) (28 days) https://github.com/yast/yast-country/pull/61 - Ensure ntp is installed (when Install Recommended Packages is off) (140 days) https://github.com/yast/yast-country/pull/55 Pending requests in repository yast-dhcp-server: - Fix "One or more selected network interfaces is not configured" (28 days) https://github.com/yast/yast-dhcp-server/pull/11 Pending requests in repository yast-installation: - Poc recover values (120 days) https://github.com/yast/yast-installation/pull/319 Pending requests in repository yast-live-installer: - Fix Symlink resolution (28 days) https://github.com/yast/yast-live-installer/pull/6 Pending requests in repository yast-snapper: - [DO NOT MERGE] Added snapshot Class with strategy pattern. (81 days) https://github.com/yast/yast-snapper/pull/40 - Cleanup and delete multiple snapshots at once (#bsc#956143). (83 days) https://github.com/yast/yast-snapper/pull/39 Pending requests in repository yast-storage: - Add option to disable swap creation (27 days) https://github.com/yast/yast-storage/pull/182 - Ported support for LVM proposal setting from SP1 to Master (4 days) https://github.com/yast/yast-storage/pull/177 - Propose /boot for encrypted root on PowerPC (63 days) https://github.com/yast/yast-storage/pull/155 Pending requests in repository yast-yast2: - Cleanup NetworkInterfaces.Read (19 days) https://github.com/yast/yast-yast2/pull/433 - Count test coverage more accurately (29 days) https://github.com/yast/yast-yast2/pull/426 Pending requests in repository yast-auth-server: - Drop ca mgm (34 days) https://github.com/yast/yast-auth-server/pull/21 Pending requests in repository yast.github.io: - Add some documentation about submit requests (88 days) https://github.com/yast/yast.github.io/pull/55 Pending requests in repository aytests-tests: - [do not merge] Installation rules (11 days) https://github.com/yast/aytests-tests/pull/12 -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

1 0

[yast-devel] Refactoring madness: chapter one
by Ancor Gonzalez Sosa 23 Feb '16

23 Feb '16

I recently invested quite some time (out of the sprint, my fault) refactoring the code of the new proposal. I didn't change the approach, the main algorithm or the produced result. So finally I changed everything for everything to stay the same. [1] I did it just because I disliked a number of things about the code organization. Code organization is a subjective question, so I though I should write a couple of mails with my reasons to make all those changes. Disclaimer: this is a highly opinionated (and veeery long) email. In this mail, I will focus on stuff that I think is kind of specific to "the Ruby way" of doing things. I will keep more general OOP stuff (like single responsibility, class dependencies, messages-driven design, etc.) out of the discussion for the time being, although it also motivated many of the introduced changes. 1) Avoid input/output arguments when possible. That is, in Ruby this puts total # => 2 total = calculator.sum(total, 3) puts total # => 5 is preferred over this puts total # => 2 calculator.sum(total, 3) puts total # => 5 Of course, in some situations this is even better... but that's not the point in my argumentation total = total.sum(3) 2) Try be as functional as possible. With methods that returns something, no matter if that thing is implemented through storage or through computation. That is, don't tell your objects what to do, tell them what you want to get. That is, this creator = PartitionCreator.new parts = creator.partitions is preferred over creator = PartitionCreator.new creator.create_partitions creator.partitions 3) Don't hesitate to create "volatile" objects and drop them, relying on the garbage collector to do its job. I will elaborate on this one. The strategy for the proposal was: use the layout read from the system (a.k.a. probed) as a starting point. Create a copy of it and run step1 on it with a set of parameters. If it failed, then discard the result and start again with a copy of "probed" trying different parameters for step1. On the other hand, if step1 was successful, consolidate that intermediate result and do a similar thing with step2 (rolling back to post-successful-step1 when needed). Good strategy... but I completely disagreed with the implementation. We have a Yast::StorageManager singleton class that acts as an interface to libstorage and that holds a catalog of devicegraphs (indexed by name). The original implementation of the proposal included the registration of three devicegraphs on that catalog: "proposal" (the working copy), "proposal_base" (the consolidated intermediate result) and "staging" (used to calculate the actions). Those devicegraphs were created as copies of any other and then passed down as I/O arguments to specialized classes performing the steps. It was usually not that obvious who had the responsibility of rolling back "proposal" to the content of "proposal_base" and so on. It was also not clear who was responsible of cleaning up the catalog after the whole process took place. My refactoring does not use that catalog at all. Moreover, it contains minimal references to that singleton object. Like here [2], in which the reference is "hidden" after a Devicegraph.probed method (in which dependency injection is used, btw). The new code creates "volatile" (in the sense of "unnamed", not indexed in a catalog) devicegraph objects. Those objects gets discarded when they are replaced with another copy resulting of a successful step (no more I/O arguments to step1) or simply when they prove to not be longer worth holding (for example, step1 failed). No globally named and registered stuff, just "a lot" of work for the garbage collector (actually trivial for the ruby GC capabilities). That's all for this mail. Sorry for the long text (and for threatening you with even more chapters), but I think that we need to agree in best practices and, even more important, on how much relevance do we want to give to them. Like "was such an investment of time in changing a perfectly working code really worth it?". I wanted to give some of my (opinionated) reasons to reply "yes". Cheers. [1] https://github.com/yast/yast-storage/pull/188 [2] https://github.com/yast/yast-storage/pull/188/files#diff-095293409316827171… -- Ancor González Sosa YaST Team at SUSE Linux GmbH -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

2 2

[yast-devel] Fwd: More mentors for YaST
by Ancor Gonzalez Sosa 23 Feb '16

23 Feb '16

The GSoC coordinators are asking for backup mentors. Please reach Christian if you plan to help with any project (or propose a new one). He will add you as backup maintainer. -------- Forwarded Message -------- Subject: More mentors for YaST Date: Mon, 15 Feb 2016 17:47:52 +0100 From: Christian Bruckmayer <cbruckmayer(a)suse.de> To: jreidinger(a)suse.cz, Ancor Gonzalez Sosa <ancor(a)suse.de>, Imobach González Sosa <igonzalezsosa(a)suse.de> Hey, we would like to add to each GSoC project at least one additional backup-mentor (at the moment we have less mentors than projects...). Is there anybody in the YaST team who we could ask to act as a backup-mentor to your already submitted projects? Thanks Christian -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

2 1

[yast-devel] yast2-rpm-handler
by opensuse.lietuviu.kalba 23 Feb '16

23 Feb '16

Hi, I created package [1], that has program to open RPM packages in YaST2 software manager (similarly as in /usr/bin/package-manager from libzypp (<=13.9) shipped with openSUSE 13.1 and earlier). Some description is FATE [2]. I have no rights to submit package directly to YaST:Head or Factory. Please, can someone submit it there? Or [better], can someone please integrate this into yast2-packager <https://github.com/yast/yast-packager> ? [1] https://build.opensuse.org/package/show/home:embar-:Lietukas/yast2-rpm-hand… [2] https://features.opensuse.org/319376 -- Regards -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

2 16

[yast-devel] Generic / Repo-specific READMEs at GitHub
by Lukas Ocilka 23 Feb '16

23 Feb '16

Hi, When Christopher was in Prague, a few of us had a brainstorming about the need of having devel-documentation placed directly in a Yast/libstorage/linuxrc repository. There are no doubts that it's one of the most important parts of projects hosted at GitHub. All their howtos mention "create README[.md]" as the very first step. In fact, we'll have new hires in the team and in such situations, good documentation is one of the most important parts. Good documentation goes from very high level to lower and lower level. We have already summarized that in Yast Team Challenges, but the summary doesn't belong to this document as it's too detailed. That's why I'm putting it here as a WIP item and deleting it from that document: --- cut --- The README should at least describe the steps needed for maintenance (for a person without expert knowledge in the area) including: * What is the project good for - what does it actually do (and how), what it cannot do (unsupported scenarios), limitations * Links to the documentation, high level and low level descriptions (e.g. RFC, wikipedia articles, man pages, project documentation, openSUSE wiki), terminology * How to fetch the source code (if something outside Git is needed it should be mentioned) * What prerequisites are needed to build the sources (external libraries, gems, scripts, …) * How to build the sources (make/rake commands…) * How to setup a testing environment (e.g. iSCSI target for iSCSI client) * How to run the code to test a change (esp. needed for non trivial projects like linuxrc) * How to run automated tests * How to build a package for submission * How to submit the package (if it needs some manual steps) * Troubleshooting (how to solve typical problems) The README should be reasonably short, if some section is too long it should be moved to a separate document (or to GitHub wiki) and linked. --- cut --- The project consists of - Taking a decision where to go - How to get there - Implementing this important documentation Those two who, according to my knowledge, were working on this were Ladislav and Christopher, but there were not the only ones and a pilot project for READMEs has been done a few months ago by Martin. It's still not finished, but moving forward. Thanks Lukas -- Lukas Ocilka, Systems Management (Yast) Team Leader SLE Department, SUSE Linux -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

4 3

[yast-devel] Pending Pull Requests
by yast-ci＠opensuse.org 23 Feb '16

23 Feb '16

This email is automatic generated from yast CI node. It lists of pull requests that have no activity more then three working days. If your module is listed, please check all pull request, why they are not merged yet. Pending requests in repository yast-autoinstallation: - Fix install rules SLE 12 GA (25 days) https://github.com/yast/yast-autoinstallation/pull/190 - [Review] Request from 'schubi2' @ 'yast/yast-autoinstallation/review_160113_bug_955878_merged_with_sp1' (39 days) https://github.com/yast/yast-autoinstallation/pull/179 Pending requests in repository yast-bootloader: - [do not merge] partial usage of config files gem (18 days) https://github.com/yast/yast-bootloader/pull/280 Pending requests in repository yast-country: - Some changes in the tumbleweed mappings (bsc#942899 and bsc#942896) (27 days) https://github.com/yast/yast-country/pull/61 - Ensure ntp is installed (when Install Recommended Packages is off) (139 days) https://github.com/yast/yast-country/pull/55 Pending requests in repository yast-dhcp-server: - Fix "One or more selected network interfaces is not configured" (27 days) https://github.com/yast/yast-dhcp-server/pull/11 Pending requests in repository yast-installation: - Poc recover values (119 days) https://github.com/yast/yast-installation/pull/319 Pending requests in repository yast-live-installer: - Fix Symlink resolution (27 days) https://github.com/yast/yast-live-installer/pull/6 Pending requests in repository yast-snapper: - [DO NOT MERGE] Added snapshot Class with strategy pattern. (80 days) https://github.com/yast/yast-snapper/pull/40 - Cleanup and delete multiple snapshots at once (#bsc#956143). (82 days) https://github.com/yast/yast-snapper/pull/39 Pending requests in repository yast-storage: - Add option to disable swap creation (26 days) https://github.com/yast/yast-storage/pull/182 - Propose /boot for encrypted root on PowerPC (62 days) https://github.com/yast/yast-storage/pull/155 Pending requests in repository yast-yast2: - Cleanup NetworkInterfaces.Read (18 days) https://github.com/yast/yast-yast2/pull/433 - Count test coverage more accurately (28 days) https://github.com/yast/yast-yast2/pull/426 Pending requests in repository yast-firewall: - Merge into Sle 12 sp1 (14 days) https://github.com/yast/yast-firewall/pull/23 Pending requests in repository yast-auth-server: - Drop ca mgm (33 days) https://github.com/yast/yast-auth-server/pull/21 Pending requests in repository yast.github.io: - Add some documentation about submit requests (87 days) https://github.com/yast/yast.github.io/pull/55 Pending requests in repository aytests-tests: - [do not merge] Installation rules (10 days) https://github.com/yast/aytests-tests/pull/12 -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

1 0

[yast-devel] Pending Pull Requests
by yast-ci＠opensuse.org 22 Feb '16

22 Feb '16

This email is automatic generated from yast CI node. It lists of pull requests that have no activity more then three working days. If your module is listed, please check all pull request, why they are not merged yet. Pending requests in repository yast-autoinstallation: - Fix install rules SLE 12 GA (24 days) https://github.com/yast/yast-autoinstallation/pull/190 - [Review] Request from 'schubi2' @ 'yast/yast-autoinstallation/review_160113_bug_955878_merged_with_sp1' (38 days) https://github.com/yast/yast-autoinstallation/pull/179 Pending requests in repository yast-bootloader: - [do not merge] partial usage of config files gem (17 days) https://github.com/yast/yast-bootloader/pull/280 Pending requests in repository yast-country: - Some changes in the tumbleweed mappings (bsc#942899 and bsc#942896) (26 days) https://github.com/yast/yast-country/pull/61 - Ensure ntp is installed (when Install Recommended Packages is off) (138 days) https://github.com/yast/yast-country/pull/55 Pending requests in repository yast-dhcp-server: - Fix "One or more selected network interfaces is not configured" (26 days) https://github.com/yast/yast-dhcp-server/pull/11 Pending requests in repository yast-installation: - Poc recover values (118 days) https://github.com/yast/yast-installation/pull/319 Pending requests in repository yast-live-installer: - Fix Symlink resolution (26 days) https://github.com/yast/yast-live-installer/pull/6 Pending requests in repository yast-snapper: - [DO NOT MERGE] Added snapshot Class with strategy pattern. (79 days) https://github.com/yast/yast-snapper/pull/40 - Cleanup and delete multiple snapshots at once (#bsc#956143). (81 days) https://github.com/yast/yast-snapper/pull/39 Pending requests in repository yast-storage: - Add option to disable swap creation (25 days) https://github.com/yast/yast-storage/pull/182 - Propose /boot for encrypted root on PowerPC (61 days) https://github.com/yast/yast-storage/pull/155 Pending requests in repository yast-yast2: - Cleanup NetworkInterfaces.Read (17 days) https://github.com/yast/yast-yast2/pull/433 - Count test coverage more accurately (27 days) https://github.com/yast/yast-yast2/pull/426 Pending requests in repository yast-firewall: - Merge into Sle 12 sp1 (13 days) https://github.com/yast/yast-firewall/pull/23 Pending requests in repository yast-auth-server: - Drop ca mgm (32 days) https://github.com/yast/yast-auth-server/pull/21 Pending requests in repository yast.github.io: - Add some documentation about submit requests (86 days) https://github.com/yast/yast.github.io/pull/55 Pending requests in repository aytests-tests: - [do not merge] Installation rules (9 days) https://github.com/yast/aytests-tests/pull/12 -- To unsubscribe, e-mail: yast-devel+unsubscribe(a)opensuse.org To contact the owner, e-mail: yast-devel+owner(a)opensuse.org

1 0