Mailinglist Archive: yast-devel (46 mails)

< Previous Next >
Fw: [yast-devel] [PATCH 1/3] Change Apparmor settings to use UI
sorry for forwarding, accidentally exclude yast-devel from reply

Begin forwarded message:

Date: Wed, 14 Jun 2017 08:49:56 +0200
From: Josef Reidinger <jreidinger@xxxxxxx>
To: Goldwyn Rodrigues <rgoldwyn@xxxxxxx>
Subject: Re: [yast-devel] [PATCH 1/3] Change Apparmor settings to use UI


On Tue, 13 Jun 2017 16:03:22 -0500
Goldwyn Rodrigues <rgoldwyn@xxxxxxx> wrote:

On 06/08/2017 02:33 AM, Josef Reidinger wrote:
On Wed, 7 Jun 2017 15:38:45 -0500
Goldwyn Rodrigues <rgoldwyn@xxxxxxx> wrote:

From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx>

This is a precursor to use aa-status json profile view.

Please note, "Configure" option is left hanging which will be filled
in the next patch.

Well, to be honest I found discussion in pull request easier to orientate
and also for discussion, but lets comment it here.

Sorry, coming from the kernel development community, I assumed a bigger
subscription here and hence more eyeballs. I would perform a pull
request in the next review cycle.

I know it is still popular in kernel or wine development, but as we live on
github, it is easier there, at least to see all comments together.
Warning: it is quite addictive :)




snip


Just note. It is quite uncommon in yast to react immediatelly in YaST UI.
Especially consider that user click on "Enable" checkbox and then press
cancel.
This should be done only after click on next, where he confirm that he
really want
to do actions.

I understand. However, I just followed the design how it exists now.
While I am open to changing the design, it may take me much longer to
learn and implement it. Let's keep it in the next round of changes, but
as a guideline, I would like to have suggestions on how it should look
like. How about yast-firewall? We will have to implement a separate
"Manage profiles". There are more changes I want to introduce there as
well such as multiple/different toggle modes.

I know that it is existing design of apparmor and very inconsistent for rest of
YaST. Reasons are historical, that this module was not developer in yast team,
but in Immunix.

I really suggest if you want to have more changes, to do it separately in
separate pull request, as it is easier to discuss it separatelly and also it
avoid situation that it need big change somewhere, which require rewrite of all
your changes.




+ if status != @service_enabled
+ Yast::Report.Error(_('Failed to change apparmor service. Please
use journal (journalctl -n -u apparmor) to diagnose'))
+ else
+ # Enable the configuration frame since everything went well
+ Yast::UI.ChangeWidget(Id(:aaEnableFrame), :Enabled, status)
+ end
end
+
end
end

-Yast::ApparmorSettingsClient.new.main
+AppArmor::ApparmorSettings.new.run

Btw, recent practive in yast is to have really small client ( like two lines
```
require "y2apparmor/clients/my_client"

Y2Apparmor::Clients::MyClient.run
```

and having code for client in lib/y2apparmor/clients/my_client.rb.
This will allow much easier unit testing of code in client ( as require do
not run code automatic )

Yes, I noticed that in the yast-journalctl tutorial. The way I divided
it is to keep all yast-apparmor interactions in lib/ and keep the rest
in clients. Does it have to be in lib/y2apparmor/clients or just lib/
would do as yast-journalctl tutorial mentions?


Well as I said, problem with clients is that it cannot be easy tested. It is
fine if you have it in lib/y2apparmor/clients and in src/clients have only thin
wrapper that call it.
--
To unsubscribe, e-mail: yast-devel+unsubscribe@xxxxxxxxxxxx
To contact the owner, e-mail: yast-devel+owner@xxxxxxxxxxxx

< Previous Next >
This Thread
  • No further messages