On Fri, Apr 1, 2016 at 10:42 AM, Craig Russell <craig.russ...@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
> c...@apache.org http://db.apache.org/jdo
- Sam Ruby
