mokasin / apw

Small and simple Awesome WM widget to control volume of Pulseaudio.
37 stars 33 forks source link

A few changes to improve readout accuracy & clean code #2

Closed zml2008 closed 11 years ago

zml2008 commented 11 years ago
mokasin commented 11 years ago

Thanks for contributing.

I've marked a few things in the code. Also:

Thanks

zml2008 commented 11 years ago

Ok, updated with your requests

On Fri, Aug 16, 2013 at 2:11 AM, mokasin notifications@github.com wrote:

Thanks for contributing.

I've marked a few things in the code. Also:

  • Please use tabs for intendation and space for line continuations
  • Would it be possible to split the commit in a feature commit and a bugfix commit (f.close() on nil)?

Thanks

— Reply to this email directly or view it on GitHubhttps://github.com/mokasin/apw/pull/2#issuecomment-22755742 .

mokasin commented 11 years ago

To call a method of a global object (or whatever you'd like to call it in Lua) does not make any sense to me. It breaks the whole idea to follow a OOP approach.

The whole used model might be not ideal. But please change it for now. Thanks.

It seems to be a feature lacking in github to pull commits but edit them before. Could do it manually though...

zml2008 commented 11 years ago

I updated the commit myself to call with self:. If it doesn't show up properly you might have to reload the page.

On Tue, Aug 20, 2013 at 7:55 AM, mokasin notifications@github.com wrote:

To call a method of a global object (or whatever you'd like to call it in Lua) does not make any sense to me. It breaks the whole idea to follow a OOP approach.

The whole used model might be not ideal. But please change it for now. Thanks.

It seems to be a feature lacking in github to pull commits but edit them before. Could do it manually though...

— Reply to this email directly or view it on GitHubhttps://github.com/mokasin/apw/pull/2#issuecomment-22950846 .

mokasin commented 11 years ago

Thanks