Skip to content

Instantly share code, notes, and snippets.

@imobachgs
Last active June 28, 2018 11:25
Show Gist options
  • Star 1 You must be signed in to star a gist
  • Fork 0 You must be signed in to fork a gist
  • Save imobachgs/d90940315bd0471ee00b2a68ba22fbe5 to your computer and use it in GitHub Desktop.
Save imobachgs/d90940315bd0471ee00b2a68ba22fbe5 to your computer and use it in GitHub Desktop.
Proposal to reorganize YaST2 code

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.

  • 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.

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/vendor_ruby (or something like that).

YaST2 reorganization proposal

From source code to installed system

Move libraries to /usr/lib64/ruby/vendor_ruby/2.1.0/ with symlinks from /usr/share/YaST2/lib.

Files/Namespaces organization

  • 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 to avoid collisions.
  • Requiring YaST2 common code:
    • yast/some_name -> legacy or Ruby bindings code
    • yast2/some_name -> new common code
  • YaST2 package:
    • Separate libraries in Yast2::UI, Yast2::Installation, Yast2::Packages, etc.
  • Modules:
    • In the sources -> src/lib/yast (because currently they live in the Yast namespace)
    • In the installed system -> /usr/lib64/vendor_ruby/2.1.0/yast/
  • Nested classes:
    • Only for private APIs.
    • Avoid cases like the storage Proposal code when possible.

Tests organization

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.

Requiring code (not discussed)

Given a y2user module:

  • require y2user will require basic classes (they should live in lib/y2users).
  • Require y2user/dialogs or y2users/whatever.
@jreidinger
Copy link

about code generators, it was there in past and result is that it become obsolete and just add to code useless stuff that are no longer valid. I prefer instead of code generator to create extension to rubocop and check if code follow yast conventions, as rubocop is easy to update. If you update code generator, you still have to cope with old code generated by old generator.

Regarding Namespaces, it is fine for me, but we still have problem that when you do something in Yast namespace, then you risk collision with Yast modules like famous Yast::String versus ::String problem. It was actually main reason for me to not use Yast namespace when possible.

changing clients dir is problem as it is used in Y2DIR and loaded by ruby-bindings, which different location it is hard to find. But it can be solvable by small wrapper that just run given client.

For me main decision is if we have Yast as whole project, that should have everything in namespace or if we want each module as own project sharing some common, so having namespace per yast module. In yast namespace there is risk of collision in Yast code, in own namespace, smaller risk of classes, but higher risk that top level namespace can collide ( but one name collision versus potential collision with a lot of libs and modules from various modules ).

@jsrain
Copy link

jsrain commented May 26, 2016

How will the move out of /usr/share/YaST2 affect the /y2update directory during installation? Will it somehow keep working also for stuff outside /usr/share/YaST2?

@lslezak
Copy link

lslezak commented Jun 20, 2016

The test for lib/yast/packager/clients/some_client.rb could live in test/lib/clients/some_client.rb

I'd prefer some_client_test.rb (or some_client_spec.rb) test file name. It makes searching (find) for clients in the sources (esp. in all YaST repositories) easier as you do not have to filter out the /test/ paths.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment