[haiku-commits] Re: haiku: hrev45344 - src/apps/activitymonitor

  • From: "Axel Dörfler" <axeld@xxxxxxxxxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Thu, 7 Mar 2013 11:12:05 +0100 (CET)

> 6cf270f: Added "Always on top" menu to the ActivityMonitor.
>   Signed-off-by: Matt Madia <mattmadia@xxxxxxxxx>

There are some style issues to point out:

> +          case kMsgAlwaysOnTop:
> +          {
> +                  SetFeel(this->IsFloating() ? B_NORMAL_WINDOW_FEEL :
> B_FLOATING_ALL_WINDOW_FEEL);

No need for this->

>  private:
> +                  BMenuItem*      fAlwaysOnTop;
> +
>                    status_t        _OpenSettings(BFile& file, uint32 mode);

Members should be where the other members are, not intermixed with method
declarations.
First come the methods, then the members.

Also, the setting is not remembered which should be changed; as is, I would say
the patch is incomplete.

Bye,
   Axel.

Other related posts: