You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@whimsical.apache.org by Craig Russell <cr...@oracle.com> on 2016/04/01 16:42:52 UTC
Comments on memstat.json.rb
Just a few questions/comments on the code.
1. entry = ASF::Person.find(@userid).members_txt(true) does some heavy lifting and then a few lines later, message = "Move #{ASF::Person.find(@userid).member_name} to #{@action}” which lifts the same weight. Could this be replaced with message = "Move #{entry.member_name} to #{@action}”?
Or is Person.find really a lightweight function?
2. There is a section “Deceased”. Would it make sense to add a few lines of code for this section?
3. Where did the @action parameter come from?
4. Could this be called directly from the command line, e.g. ruby views/actions/memstat clr emeritus
Craig
Craig L Russell
Secretary, Apache Software Foundation
clr@apache.org http://db.apache.org/jdo
Re: Comments on memstat.json.rb
Posted by Sam Ruby <ru...@intertwingly.net>.
On Fri, Apr 1, 2016 at 11:45 AM, sebb <se...@gmail.com> wrote:
> On 1 April 2016 at 16:09, Sam Ruby <ru...@intertwingly.net> wrote:
>> On Fri, Apr 1, 2016 at 10:42 AM, Craig Russell <cr...@oracle.com> wrote:
>>
>>> 4. Could this be called directly from the command line, e.g. ruby views/actions/memstat clr emeritus
>>
>> Directly? No. But it is quite possible to write a script that sets a
>> few instance variables and then reads and evals the script. The tests
>> for the board agenda tool do this:
>>
>> https://github.com/apache/whimsy/blob/master/www/board/agenda/spec/actions_spec.rb#L40
>
> Is it not possible to add a section of the form:
>
> if __FILE__ == $0
> @uid = ARGV[0]
> @state = ARGV[1]
> eval(File.read(__FILE__)) # or some other invocation method
> end
That indeed could be added to the top of the file, without the eval.
- Sam Ruby
Re: Comments on memstat.json.rb
Posted by sebb <se...@gmail.com>.
On 1 April 2016 at 16:09, Sam Ruby <ru...@intertwingly.net> wrote:
> On Fri, Apr 1, 2016 at 10:42 AM, Craig Russell <cr...@oracle.com> wrote:
>> Just a few questions/comments on the code.
>>
>> 1. entry = ASF::Person.find(@userid).members_txt(true) does some heavy lifting and then a few lines later, message = "Move #{ASF::Person.find(@userid).member_name} to #{@action}” which lifts the same weight. Could this be replaced with message = "Move #{entry.member_name} to #{@action}”?
>
> person.members_txt() returns a string, so entry.member_name won't work.
>
>> Or is Person.find really a lightweight function?
>
> ASF::Person.find caches its results, so subsequent calls are light weight.
>
> If you feel so inclined, feel free to refactor this to person =
> ASF::Person.find(@userid); and then change the two places were
> Person.find is called to use this value.
>
>> 2. There is a section “Deceased”. Would it make sense to add a few lines of code for this section?
>
> It would not be difficult to do. Go for it!
>
>> 3. Where did the @action parameter come from?
>
> It comes from the form. See:
>
> https://github.com/apache/whimsy/blob/ef8a40770cbba9da72da0bfacb45cf334ab248ec/www/roster/views/committer.js.rb#L187
>
> If you were to add other buttons, the name/value pairs on those
> buttons (or input fields, or textareas, etc), will be used to set
> instance variables (i.e., variables whose names start with an '@') to
> the associated value.
>
> Feel free to experiment. If you were to create a button and push that
> change, it would only show up to people listed in the asf-secretary
> LDAP service:
>
> https://whimsy.apache.org/roster/group/asf-secretary
>
>> 4. Could this be called directly from the command line, e.g. ruby views/actions/memstat clr emeritus
>
> Directly? No. But it is quite possible to write a script that sets a
> few instance variables and then reads and evals the script. The tests
> for the board agenda tool do this:
>
> https://github.com/apache/whimsy/blob/master/www/board/agenda/spec/actions_spec.rb#L40
Is it not possible to add a section of the form:
if __FILE__ == $0
@uid = ARGV[0]
@state = ARGV[1]
eval(File.read(__FILE__)) # or some other invocation method
end
This is done in the monitors/*.rb files
> It should be possible to write proper unit tests for this function,
> given dummy test data for members.txt. If this is of interest, I can
> set that up over the weekend, and you could add to it.
>
> The board agenda tests can test for server rendering, and can also
> make use of phantomjs which runs a headless webkit based browser and
> can be used to test javascript.
>
> Tests for pure server functions (without rendering) run very fast.
> When rendering is involved, it is noticeably slower, but not too bad.
> Tests that involve the creation and teardown of a browser are even
> slower, to the point where such tests are generally only worth it for
> non-trivial operations. I wouldn't, for example, be inclined to write
> a test for showing the emeritus/deceased buttons.
>
>> Craig
>>
>> Craig L Russell
>> Secretary, Apache Software Foundation
>> clr@apache.org http://db.apache.org/jdo
>
> - Sam Ruby
Re: Comments on memstat.json.rb
Posted by Craig Russell <cr...@oracle.com>.
I just realized I was not on the whimsy dev list so I missed most of this discussion. :(
> On Apr 2, 2016, at 5:24 PM, Sam Ruby <ru...@intertwingly.net> wrote:
>
> On Fri, Apr 1, 2016 at 11:43 AM, Sam Ruby <ru...@intertwingly.net> wrote:
>>
>> Just a heads up: I'm looking at adding support for a "dry run" of
>> these scripts, which likely will result in some change to the script
>> itself, and may need additional setup by wrapper scripts.
>
> The /roster/committer/* pages now support dry runs of changes. Double
> click on on a light green area to show a form; make the updates you
> wish, and then option-click on Macs (control-click everwhere else) on
> the relevant button. The results will come back in JSON form. For
> LDAP changes, they will show you the previous value, for SVN changes
> you will see a transcript of commands issued and their responses.
>
> You will also see other information, including a JSON representation
> of the committer page.
>
> - - -
>
> This tool can be run locally assuming you have your .whimsy file set
> up and LDAP configured. See
> https://github.com/apache/whimsy/blob/master/DEVELOPMENT.md#setup
>
> The commands you need to run a local server can be found in
> https://github.com/apache/whimsy/blob/master/www/roster/README
>
> The server will automatically restart whenever it detects a code
> change. You mentioned adding an action to move a member to
> deceased... that sounds like a relatively straightforward change that
> can be developed and tested locally. To make it even easier, I've
> split up the file that shows the page, which means that only two files
> would need to be updated for this change. For the client side:
>
> https://github.com/apache/whimsy/blob/master/www/roster/views/person/memstat.js.rb
>
> and for the server side:
>
> https://github.com/apache/whimsy/blob/master/www/roster/views/actions/memstat.json.rb
>
> - Sam Ruby
Craig L Russell
Architect
craig.russell@oracle.com
P.S. A good JDO? O, Gasp!
Re: Comments on memstat.json.rb
Posted by Sam Ruby <ru...@intertwingly.net>.
On Fri, Apr 1, 2016 at 11:43 AM, Sam Ruby <ru...@intertwingly.net> wrote:
>
> Just a heads up: I'm looking at adding support for a "dry run" of
> these scripts, which likely will result in some change to the script
> itself, and may need additional setup by wrapper scripts.
The /roster/committer/* pages now support dry runs of changes. Double
click on on a light green area to show a form; make the updates you
wish, and then option-click on Macs (control-click everwhere else) on
the relevant button. The results will come back in JSON form. For
LDAP changes, they will show you the previous value, for SVN changes
you will see a transcript of commands issued and their responses.
You will also see other information, including a JSON representation
of the committer page.
- - -
This tool can be run locally assuming you have your .whimsy file set
up and LDAP configured. See
https://github.com/apache/whimsy/blob/master/DEVELOPMENT.md#setup
The commands you need to run a local server can be found in
https://github.com/apache/whimsy/blob/master/www/roster/README
The server will automatically restart whenever it detects a code
change. You mentioned adding an action to move a member to
deceased... that sounds like a relatively straightforward change that
can be developed and tested locally. To make it even easier, I've
split up the file that shows the page, which means that only two files
would need to be updated for this change. For the client side:
https://github.com/apache/whimsy/blob/master/www/roster/views/person/memstat.js.rb
and for the server side:
https://github.com/apache/whimsy/blob/master/www/roster/views/actions/memstat.json.rb
- Sam Ruby
Re: Comments on memstat.json.rb
Posted by Sam Ruby <ru...@intertwingly.net>.
On Fri, Apr 1, 2016 at 11:09 AM, Sam Ruby <ru...@intertwingly.net> wrote:
> On Fri, Apr 1, 2016 at 10:42 AM, Craig Russell <cr...@oracle.com> wrote:
>
>> 4. Could this be called directly from the command line, e.g. ruby views/actions/memstat clr emeritus
>
> Directly? No. But it is quite possible to write a script that sets a
> few instance variables and then reads and evals the script. The tests
> for the board agenda tool do this:
>
> https://github.com/apache/whimsy/blob/master/www/board/agenda/spec/actions_spec.rb#L40
>
> It should be possible to write proper unit tests for this function,
> given dummy test data for members.txt. If this is of interest, I can
> set that up over the weekend, and you could add to it.
Just a heads up: I'm looking at adding support for a "dry run" of
these scripts, which likely will result in some change to the script
itself, and may need additional setup by wrapper scripts.
- Sam Ruby
Re: Comments on memstat.json.rb
Posted by Sam Ruby <ru...@intertwingly.net>.
On Fri, Apr 1, 2016 at 10:42 AM, Craig Russell <cr...@oracle.com> wrote:
> Just a few questions/comments on the code.
>
> 1. entry = ASF::Person.find(@userid).members_txt(true) does some heavy lifting and then a few lines later, message = "Move #{ASF::Person.find(@userid).member_name} to #{@action}” which lifts the same weight. Could this be replaced with message = "Move #{entry.member_name} to #{@action}”?
person.members_txt() returns a string, so entry.member_name won't work.
> Or is Person.find really a lightweight function?
ASF::Person.find caches its results, so subsequent calls are light weight.
If you feel so inclined, feel free to refactor this to person =
ASF::Person.find(@userid); and then change the two places were
Person.find is called to use this value.
> 2. There is a section “Deceased”. Would it make sense to add a few lines of code for this section?
It would not be difficult to do. Go for it!
> 3. Where did the @action parameter come from?
It comes from the form. See:
https://github.com/apache/whimsy/blob/ef8a40770cbba9da72da0bfacb45cf334ab248ec/www/roster/views/committer.js.rb#L187
If you were to add other buttons, the name/value pairs on those
buttons (or input fields, or textareas, etc), will be used to set
instance variables (i.e., variables whose names start with an '@') to
the associated value.
Feel free to experiment. If you were to create a button and push that
change, it would only show up to people listed in the asf-secretary
LDAP service:
https://whimsy.apache.org/roster/group/asf-secretary
> 4. Could this be called directly from the command line, e.g. ruby views/actions/memstat clr emeritus
Directly? No. But it is quite possible to write a script that sets a
few instance variables and then reads and evals the script. The tests
for the board agenda tool do this:
https://github.com/apache/whimsy/blob/master/www/board/agenda/spec/actions_spec.rb#L40
It should be possible to write proper unit tests for this function,
given dummy test data for members.txt. If this is of interest, I can
set that up over the weekend, and you could add to it.
The board agenda tests can test for server rendering, and can also
make use of phantomjs which runs a headless webkit based browser and
can be used to test javascript.
Tests for pure server functions (without rendering) run very fast.
When rendering is involved, it is noticeably slower, but not too bad.
Tests that involve the creation and teardown of a browser are even
slower, to the point where such tests are generally only worth it for
non-trivial operations. I wouldn't, for example, be inclined to write
a test for showing the emeritus/deceased buttons.
> Craig
>
> Craig L Russell
> Secretary, Apache Software Foundation
> clr@apache.org http://db.apache.org/jdo
- Sam Ruby