[yast-devel] YaST2 reorganization discussion
Hi all, During the past openSUSE Conference, some members of the YaST team (Áncor, Christopher, HuHa, Knut, Josef and myself -alphabetical order, excuse me if I forgot anyone) discussed about YaST2 code reorganization. The outcomes of such a discussion are summarized in this document: https:// gist.github.com/imobach/d90940315bd0471ee00b2a68ba22fbe5#proposal Obviously the discussion is still open so, please, feel free to comment and bring up new ideas. Regards, Imo -- Imobach González Sosa YaST Team at SUSE LINUX GmbH
On Mon, 27 Jun 2016 06:59:13 +0100 Imobach González Sosa <igonzalezsosa@suse.de> wrote:
Hi all,
During the past openSUSE Conference, some members of the YaST team (Áncor, Christopher, HuHa, Knut, Josef and myself -alphabetical order, excuse me if I forgot anyone) discussed about YaST2 code reorganization.
The outcomes of such a discussion are summarized in this document: https:// gist.github.com/imobach/d90940315bd0471ee00b2a68ba22fbe5#proposal
Obviously the discussion is still open so, please, feel free to comment and bring up new ideas.
Regards, Imo
OK, few notes: - I think it is good idea to also use oppurtinity when you are in Prague and discuss it also there, maybe more ideas or previously not seen issues appear - problem with UI is not that it is too generic, but that it often use Yast::UI which creates nasty name collision and confusion if Y2User::UI will use Yast::UI - in the end we discuss that such require layer is maybe not so useful and can cause some confusion, so maybe if we still use import, it indicate we use some legacy module and not common ruby library - regarding nested classes we also discussed usage of rubocop to detect it and report it as issue if it is not marked as private - regarding move of lib to /usr/share/Yast2/lib there is still question what to do with Y2UPDATE? maybe add it by default to ruby load PATH? Josef
On Mon, Jun 27, 2016 at 06:59:13AM +0100, Imobach González Sosa wrote:
During the past openSUSE Conference, some members of the YaST team (Áncor, Christopher, HuHa, Knut, Josef and myself -alphabetical order, excuse me if I forgot anyone) discussed about YaST2 code reorganization.
The outcomes of such a discussion are summarized in this document: https:// gist.github.com/imobach/d90940315bd0471ee00b2a68ba22fbe5#proposal
Obviously the discussion is still open so, please, feel free to comment and bring up new ideas.
Have you considered going all the way and packaging our repos as gems? You know, gemspec, gem2rpm, rpm. Maybe the parts with C++, cough Perl cough, would be harder, but it should be easy for pure Ruby, once we conform to the conventions as proposed above. I think the benefit would be that bundler could set up things that we now need to do in VMs. -- Martin Vidner, YaST Team http://en.opensuse.org/User:Mvidner Kuracke oddeleni v restauraci je jako fekalni oddeleni v bazenu
On Mon, 27 Jun 2016 16:12:05 +0200 Martin Vidner <mvidner@suse.cz> wrote:
On Mon, Jun 27, 2016 at 06:59:13AM +0100, Imobach González Sosa wrote:
During the past openSUSE Conference, some members of the YaST team (Áncor, Christopher, HuHa, Knut, Josef and myself -alphabetical order, excuse me if I forgot anyone) discussed about YaST2 code reorganization.
The outcomes of such a discussion are summarized in this document: https:// gist.github.com/imobach/d90940315bd0471ee00b2a68ba22fbe5#proposal
Obviously the discussion is still open so, please, feel free to comment and bring up new ideas.
Have you considered going all the way and packaging our repos as gems? You know, gemspec, gem2rpm, rpm.
Maybe the parts with C++, cough Perl cough, would be harder, but it should be easy for pure Ruby, once we conform to the conventions as proposed above.
I think the benefit would be that bundler could set up things that we now need to do in VMs.
Yes, we consider it and reason why not is still same. yast module contain beside lib also other parts like clients, modules, autoyast schemas and others, which cannot be packaged as gem, as we cannot get them into correct location. If you find reasonable way, to package it into gem, we can reopen discussion. Josef
On Tue, Jun 28, 2016 at 08:00:45AM +0200, Josef Reidinger wrote:
On Mon, 27 Jun 2016 16:12:05 +0200 Martin Vidner <mvidner@suse.cz> wrote:
Have you considered going all the way and packaging our repos as gems? You know, gemspec, gem2rpm, rpm.
Maybe the parts with C++, cough Perl cough, would be harder, but it should be easy for pure Ruby, once we conform to the conventions as proposed above.
I think the benefit would be that bundler could set up things that we now need to do in VMs.
Yes, we [have] consider[ed] it and [the] reason why not [use gems] is still [the] same. A yast module contain beside lib also other parts like clients, modules, autoyast schemas and others, which cannot be packaged as gem, as we cannot get them into correct location.
Yes, but these are all part of a YaST API that we control, so we could amend core and ruby-bindings to look in new gem-style paths. Are there paths that we do not control? - desktop files - ...? -- Martin Vidner, YaST Team http://en.opensuse.org/User:Mvidner Kuracke oddeleni v restauraci je jako fekalni oddeleni v bazenu
On Tue, 28 Jun 2016 11:35:26 +0200 Martin Vidner <mvidner@suse.cz> wrote:
On Tue, Jun 28, 2016 at 08:00:45AM +0200, Josef Reidinger wrote:
On Mon, 27 Jun 2016 16:12:05 +0200 Martin Vidner <mvidner@suse.cz> wrote:
Have you considered going all the way and packaging our repos as gems? You know, gemspec, gem2rpm, rpm.
Maybe the parts with C++, cough Perl cough, would be harder, but it should be easy for pure Ruby, once we conform to the conventions as proposed above.
I think the benefit would be that bundler could set up things that we now need to do in VMs.
Yes, we [have] consider[ed] it and [the] reason why not [use gems] is still [the] same. A yast module contain beside lib also other parts like clients, modules, autoyast schemas and others, which cannot be packaged as gem, as we cannot get them into correct location.
Yes, but these are all part of a YaST API that we control, so we could amend core and ruby-bindings to look in new gem-style paths.
Are there paths that we do not control? - desktop files - ...?
- fillup scripts Other then that probably not.
I am quoting the markdown here:
# YaST2 reorganization
## Problems
### Namespaces
We see these problems in this area:
* Polluting the Ruby top-level namespaces with names as generic as Installation, Packages or UI. * Inconsistencies. * Collisions (e.g. Storage is already a module and cannot be used as a namespace).
Let's see some examples:
* `::Registration::UI::Dialog` vs `::UI::InstallationDialog` * `::Installation::FinishClient` vs `Yast::PkgFinishClient`
You can see a complete example in [PkgFinishClient](https://github.com/yast/yast-packager/blob/a0b38c046e6e4086a986047d0d7cd5d15...).
* PkgFinishClient lives in Yast namespace (although is located in `lib/packages/clients/pkg_finish.rb`; more on that, later). * It inherits from `::Installation::FinishClient`, which live outside the YaST namespace (polluting the top-level one).
### Source code layout
About source code layout, situation is not that bad but it can be improved. Sometimes is not straightforward to find where a class/module is defined (it's easy enough using grep or the Github search, but I'd prefer a more consistent/predictible way).
Although it's not mandatory, using a single class/module per file and naming the file as the class/module (but replacing CamelCase with snake_case) is the preferred option within the [Ruby community](https://github.com/bbatsov/ruby-style-guide#one-class-per-file).
Going back to `PkgFinishClient`, it should live in `pkg_finish_client.rb`. And a `Yast::SomeModule::Clients::SomeClient` class should be defined in `lib/yast/somemodule/clients/some_client.rb`.
### Requiring code
Steffen pointed out that if he requires `yast/storage`, he would expect all storage code to be loaded. And he's right. If you want to load only a class/file, you can require `yast/storage/some_class`.
### Installed code
In an installed system, all YaST2 Ruby code lives in `/usr/share/YaST2`. That's fine for things like modules (`/usr/share/YaST2/modules`) which are quite YaST-specific. But from a Ruby developer point of view, libraries should live in `/usr/lib64/ruby/yast` (or something like that).
## Proposal
### Files/Namespaces organization
IMHO the big questions are: 1) Namespacing 2) Migration from the current state File organization should be a straightforward application of the Ruby conventions to the namespacing, right?
* Use `Y2Users`, `Y2Storage`, etc. as namespace for each module to avoid collisions. Code should be organized under: * `Y2Users::Dialogs::SomeName` * `Y2Users::Clients::SomeName` * `Y2Users::Widgets::SomeName` * and so on * Avoid `UI` namespace as it sounds too 'general' (use `Dialogs` or `Widgets`).
* Write a layer so, when requiring a library, it tries to find the constant and, if it's not found, ask the component system. The goal is to finally avoid `Yast.import`.
So for Arch, for example: Before: Yast.import "Arch" # -> /usr/share/YaST2/modules/Arch.rb After require "yast/arch" # -> .../lib/yast/arch.rb # and ruby-bindings are adapted so that Perl # can still find it with Import
* Requiring YaST2 common code: * `yast/some-name` -> legacy or Ruby bindings code
So what exactly would be the namespaces+paths for a legacy - module - include - client ?
* `yast2/some-name` -> new code * Nested classes: * Only for private APIs.
What does this mean? Above I see 3 nested classes, `Y2Users::*::SomeName`
* Avoid cases like the storage `Proposal` code when possible.
Sorry, I don't know this case. What is it and why is it bad?
### Tests organization
```ruby spec clients/ dialogs/ widgets/ module_mymodule_spec.rb client_myclient_spec.rb ```
* The intention behind `module_mymodule_spec.rb` and `client_myclient_spec.rb` naming is to make clear that they should disappear in the future.
### From source code to installed system
Move libraries to `/usr/lib64/ruby/vendor` with symlinks from `/usr/share/YaST2/lib`.
### Requiring code (not discussed)
If we require `yast/packager`, maybe we don't want all the dialogs, clients and that kind of stuff to be loaded. We could also define a `yast/packager/common` file to load only common code (libraries).
-- Martin Vidner, YaST Team http://en.opensuse.org/User:Mvidner Kuracke oddeleni v restauraci je jako fekalni oddeleni v bazenu
On Mon, 27 Jun 2016 16:36:53 +0200 Martin Vidner <mvidner@suse.cz> wrote:
I am quoting the markdown here:
# YaST2 reorganization
1) Namespacing
we write proposal below for namespacing
2) Migration from the current state
we agreed we would like to start with our target state and then move files when it is touched or when new ones will be written, so no big bang as even path is part of API ( you require it )
File organization should be a straightforward application of the Ruby conventions to the namespacing, right?
yes
* Use `Y2Users`, `Y2Storage`, etc. as namespace for each module to avoid collisions. Code should be organized under: * `Y2Users::Dialogs::SomeName` * `Y2Users::Clients::SomeName` * `Y2Users::Widgets::SomeName` * and so on * Avoid `UI` namespace as it sounds too 'general' (use `Dialogs` or `Widgets`).
* Write a layer so, when requiring a library, it tries to find the constant and, if it's not found, ask the component system. The goal is to finally avoid `Yast.import`.
So for Arch, for example: Before: Yast.import "Arch" # -> /usr/share/YaST2/modules/Arch.rb After require "yast/arch" # -> .../lib/yast/arch.rb # and ruby-bindings are adapted so that Perl # can still find it with Import
yes, if we agreed we want it. Drawback is that actually modules are not so common ruby classes, so it can also cause some confusion, so maybe import is better to emphasis is not common ruby code in common location?
* Requiring YaST2 common code: * `yast/some-name` -> legacy or Ruby bindings code
So what exactly would be the namespaces+paths for a legacy - module - include - client ?
still same Yast as before, no changes there. We just agreed that for code in lib, we prefer to not use Yast namespace, as there is collisions with ruby. So we use different namespace.
* `yast2/some-name` -> new code * Nested classes: * Only for private APIs.
What does this mean? Above I see 3 nested classes, `Y2Users::*::SomeName`
no, it is not classes, but namespace ( a.k.a ruby modules ). module A module B class C ... is fine as it is just namespaced class module A class B .... class C ... is nested class C and C should be only private for B usage. You can force to be it really private with Module#private_constant applied on that class
* Avoid cases like the storage `Proposal` code when possible.
Sorry, I don't know this case. What is it and why is it bad?
Well, basically what was wrong is that proposal have nested classes ( I do not use inner class as it have slightly different behavior like sharing context, which is not possible in ruby ). And problem is that part of nested classes is for private usage only and part is for public usage like ProposalConfiguration should be used publicly and SpaceCalcular is private class only intended to use for Proposal class. So it create confusion what is public class and what is private class. So we agreed on this simple rule, that nested classes are always private and is not part of public API. Josef
On Tue, Jun 28, 2016 at 08:11:36AM +0200, Josef Reidinger wrote:
On Mon, 27 Jun 2016 16:36:53 +0200 Martin Vidner <mvidner@suse.cz> wrote:
I am quoting the markdown here:
# YaST2 reorganization
1) Namespacing
we write proposal below for namespacing
2) Migration from the current state
we agreed we would like to start with our target state and then move files when it is touched or when new ones will be written, so no big bang as even path is part of API ( you require it )
OK. The risk is a halfway change and even more confusion unless rules are documented clearly and enforced well. An alternative is to prepare and execute a big bang like we did with YCP->Ruby.
File organization should be a straightforward application of the Ruby conventions to the namespacing, right?
yes
* Use `Y2Users`, `Y2Storage`, etc. as namespace for each module to avoid collisions. Code should be organized under: * `Y2Users::Dialogs::SomeName` * `Y2Users::Clients::SomeName` * `Y2Users::Widgets::SomeName` * and so on
So according to the rule "yast2 for new code below(*), will it be Y2Users::Dialogs::SomeName in lib/y2_users/dialogs/some_name.rb or Yast2::Y2Users::Dialogs::SomeName in lib/yast2/y2_users/dialogs/some_name.rb ? (for reference, $ ruby -r active_support/core_ext/string/conversions -e 'puts "Y2Users".underscore' y2_users )
* Avoid `UI` namespace as it sounds too 'general' (use `Dialogs` or `Widgets`).
* Write a layer so, when requiring a library, it tries to find the constant and, if it's not found, ask the component system. The goal is to finally avoid `Yast.import`.
So for Arch, for example: Before: Yast.import "Arch" # -> /usr/share/YaST2/modules/Arch.rb After require "yast/arch" # -> .../lib/yast/arch.rb # and ruby-bindings are adapted so that Perl # can still find it with Import
yes, if we agreed we want it. Drawback is that actually modules are not so common ruby classes, so it can also cause some confusion, so maybe import is better to emphasis is not common ruby code in common location?
That is not a sufficient reason to prefer `Yast.import` over `require`. To summarize what is weird about yast module, to an unsuspecting Ruby developer: --- Before: Yast.import "Arch" puts "system z" if Yast::Arch.s390 After: require "yast/arch" puts "system z" if Yast::Arch.s390 Classes in the Yast:: namespace (do not confuse with Yast2:: (?)) are all named with a ...Class suffix and have Yast::Module as a superclass. What looks like class methods all over the place are in fact instance methods on global singleton instances of these classes: class Yast::ArchClass < Yast::Module def s390 end end Yast::Arch = Yast::ArchClass.new This is because YCP, the previous implementation language of YaST, only supported a single instance of a module. ---
* Requiring YaST2 common code: * `yast/some-name` -> legacy or Ruby bindings code
So what exactly would be the namespaces+paths for a legacy - module - include - client ?
still same Yast as before, no changes there.
IMHO wrong: - prevents gem packaging - incompatible with `require "yast/arch"`
We just agreed that for code in lib, we prefer to not use Yast namespace, as there is collisions with ruby. So we use different namespace.
(*) see above
* `yast2/some-name` -> new code * Nested classes: * Only for private APIs.
What does this mean? Above I see 3 nested classes, `Y2Users::*::SomeName`
no, it is not classes, but namespace ( a.k.a ruby modules ). [...]
OK, I see.
* Avoid cases like the storage `Proposal` code when possible.
Sorry, I don't know this case. What is it and why is it bad?
Well, basically what was wrong is that proposal have nested classes ( I do not use inner class as it have slightly different behavior like sharing context, which is not possible in ruby ). And problem is that part of nested classes is for private usage only and part is for public usage like ProposalConfiguration should be used publicly and SpaceCalcular is private class only intended to use for Proposal class. So it create confusion what is public class and what is private class. So we agreed on this simple rule, that nested classes are always private and is not part of public API.
OK -- Martin Vidner, YaST Team http://en.opensuse.org/User:Mvidner Kuracke oddeleni v restauraci je jako fekalni oddeleni v bazenu
On Tue, 28 Jun 2016 12:01:05 +0200 Martin Vidner <mvidner@suse.cz> wrote:
On Tue, Jun 28, 2016 at 08:11:36AM +0200, Josef Reidinger wrote:
On Mon, 27 Jun 2016 16:36:53 +0200 Martin Vidner <mvidner@suse.cz> wrote:
I am quoting the markdown here:
# YaST2 reorganization
1) Namespacing
we write proposal below for namespacing
2) Migration from the current state
we agreed we would like to start with our target state and then move files when it is touched or when new ones will be written, so no big bang as even path is part of API ( you require it )
OK. The risk is a halfway change and even more confusion unless rules are documented clearly and enforced well. An alternative is to prepare and execute a big bang like we did with YCP->Ruby.
I think we should start with agile approach and try it for few modules and if it works without issues, use it everywhere.
File organization should be a straightforward application of the Ruby conventions to the namespacing, right?
yes
* Use `Y2Users`, `Y2Storage`, etc. as namespace for each module to avoid collisions. Code should be organized under: * `Y2Users::Dialogs::SomeName` * `Y2Users::Clients::SomeName` * `Y2Users::Widgets::SomeName` * and so on
So according to the rule "yast2 for new code below(*), will it be Y2Users::Dialogs::SomeName in lib/y2_users/dialogs/some_name.rb or Yast2::Y2Users::Dialogs::SomeName in lib/yast2/y2_users/dialogs/some_name.rb ?
the first one. yast2 is reserver for yast-yast2. We simply find better to have module separated namespaces, so there is no risk of collisions and Yast2::Y2Users::Dialogs::SomeName is too long to my taste, it almost fits whole line even if used without parameters.
(for reference, $ ruby -r active_support/core_ext/string/conversions -e 'puts "Y2Users".underscore' y2_users )
* Avoid `UI` namespace as it sounds too 'general' (use `Dialogs` or `Widgets`).
* Write a layer so, when requiring a library, it tries to find the constant and, if it's not found, ask the component system. The goal is to finally avoid `Yast.import`.
So for Arch, for example: Before: Yast.import "Arch" # -> /usr/share/YaST2/modules/Arch.rb After require "yast/arch" # -> .../lib/yast/arch.rb # and ruby-bindings are adapted so that Perl # can still find it with Import
yes, if we agreed we want it. Drawback is that actually modules are not so common ruby classes, so it can also cause some confusion, so maybe import is better to emphasis is not common ruby code in common location?
That is not a sufficient reason to prefer `Yast.import` over `require`.
Well, ancor object it, so I expect we are probably too affected by YCP and he is ruby newcomer, so easier for him to see suspicious things. But if there is agreement, then as I mentioned before, it is not problem for me to change yast to modify require ( also note that it always have to be require "yast" and then require modules with require "yast/arch"
To summarize what is weird about yast module, to an unsuspecting Ruby developer:
--- Before:
Yast.import "Arch" puts "system z" if Yast::Arch.s390
After: require "yast/arch" puts "system z" if Yast::Arch.s390
Classes in the Yast:: namespace (do not confuse with Yast2:: (?)) are all named with a ...Class suffix and have Yast::Module as a superclass. What looks like class methods all over the place are in fact instance methods on global singleton instances of these classes:
class Yast::ArchClass < Yast::Module def s390 end end Yast::Arch = Yast::ArchClass.new
This is because YCP, the previous implementation language of YaST, only supported a single instance of a module.
in ruby world singleton instances looks like one with Singleton mixin, so it have instance method ;)
---
* Requiring YaST2 common code: * `yast/some-name` -> legacy or Ruby bindings code
So what exactly would be the namespaces+paths for a legacy - module - include - client ?
still same Yast as before, no changes there.
IMHO wrong: - prevents gem packaging - incompatible with `require "yast/arch"`
ugh, why? I do not get what is wrong with keeping modules, includes and clients in Yast namespace? why it prevents gem packaging? and why it is incompatible? or do you mean to move them to lib directory under yast dir? But how to handle if some modules have Bootloader as client, module and also include?
We just agreed that for code in lib, we prefer to not use Yast namespace, as there is collisions with ruby. So we use different namespace.
(*) see above
still do not get it.
On 28.06.2016 15:03, Josef Reidinger wrote:
OK. The risk is a halfway change and even more confusion unless rules
are documented clearly and enforced well. An alternative is to prepare and execute a big bang like we did with YCP->Ruby. I think we should start with agile approach and try it for few modules and if it works without issues, use it everywhere.
And if not, revert it? This would result in that halfway change Martin mentioned. Kind regards -- Stefan Hundhammer <shundhammer@suse.de> YaST Developer SUSE Linux GmbH GF: Felix Imendörffer, Jane Smithard, Graham Norton; HRB 21284 (AG Nürnberg) Maxfeldstr. 5, 90409 Nürnberg, Germany -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
On Tue, 28 Jun 2016 18:03:47 +0200 Stefan Hundhammer <shundhammer@suse.de> wrote:
On 28.06.2016 15:03, Josef Reidinger wrote:
OK. The risk is a halfway change and even more confusion unless rules
are documented clearly and enforced well. An alternative is to prepare and execute a big bang like we did with YCP->Ruby. I think we should start with agile approach and try it for few modules and if it works without issues, use it everywhere.
And if not, revert it?
This would result in that halfway change Martin mentioned.
Kind regards
And if it doesn't work, then find better way and try it on given subset of modules until we find working solution. It is like when you testing drugs on testing examples and if it works, then apply globally. -- To unsubscribe, e-mail: yast-devel+unsubscribe@opensuse.org To contact the owner, e-mail: yast-devel+owner@opensuse.org
participants (4)
-
Imobach González Sosa -
Josef Reidinger -
Martin Vidner -
Stefan Hundhammer