Open GoogleCodeExporter opened 9 years ago
Story should be the component number, period, followed by story number. Please
update your summary so it is consistent with everyone else.
Original comment by guc@sakr.me
on 10 May 2012 at 9:47
Original comment by omar.sae...@gmail.com
on 10 May 2012 at 11:03
Original comment by omar.sae...@gmail.com
on 10 May 2012 at 11:06
Original comment by omar.sae...@gmail.com
on 10 May 2012 at 12:14
Original comment by omar.sae...@gmail.com
on 12 May 2012 at 7:18
This issue was updated by revision r1195.
added integration tests
Original comment by omar.sae...@gmail.com
on 13 May 2012 at 7:10
Original comment by omar.sae...@gmail.com
on 13 May 2012 at 7:13
This issue was updated by revision r1382.
status inprogress
added int_toggle method and removed user_add_interests
Original comment by omar.sae...@gmail.com
on 14 May 2012 at 10:50
This issue was updated by revision r1383.
status inprogress
added method toggle_interests
Original comment by omar.sae...@gmail.com
on 14 May 2012 at 10:52
This issue was updated by revision r1385.
status inprogress
added view
Original comment by omar.sae...@gmail.com
on 14 May 2012 at 10:54
This issue was updated by revision r1386.
status inprogress
modified routes to toggle and int_toggle methods
Original comment by omar.sae...@gmail.com
on 14 May 2012 at 10:56
This issue was updated by revision r1387.
status inprogress
added back button in case no interests on system
Original comment by omar.sae...@gmail.com
on 14 May 2012 at 11:05
Original comment by omar.sae...@gmail.com
on 14 May 2012 at 11:09
CC: UML reviewer
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 12:24
Success scenarios:
A list of button of interests will appear with added interests (green) and the
rest (Red) on adding interest its color changes to green & on deleting its
color changed to red
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 12:30
This issue was updated by revision r1413.
added UML
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 12:42
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 12:47
This issue was updated by revision r1419.
modified no interests on system condition
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 1:42
This issue was updated by revision r1425.
modified documentation
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 3:10
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 6:02
This issue was updated by revision r1494.
fixed error (code added inside int_toggle method)
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 9:06
This issue was updated by revision r1503.
modified in no interests on system
Original comment by omar.sae...@gmail.com
on 15 May 2012 at 9:34
This issue was updated by revision r1531.
changed main feed button color
Original comment by omar.sae...@gmail.com
on 16 May 2012 at 10:37
This issue was updated by revision r1552.
changed main feed button
Original comment by omar.sae...@gmail.com
on 16 May 2012 at 5:17
This issue was updated by revision r1557.
added flash if no selected interests
Original comment by omar.sae...@gmail.com
on 16 May 2012 at 6:25
This issue was updated by revision r1558.
added flash if no selected interests
Original comment by omar.sae...@gmail.com
on 16 May 2012 at 6:25
This issue was updated by revision r1647.
added log message
Original comment by omar.sae...@gmail.com
on 17 May 2012 at 6:25
This issue was updated by revision r1723.
added "blocked" to interest name if blocked
Original comment by omar.sae...@gmail.com
on 18 May 2012 at 1:20
This issue was updated by revision r1724.
added is_blocked to check if interest is blocked
Original comment by omar.sae...@gmail.com
on 18 May 2012 at 1:24
This issue was updated by revision r1762.
added two tests and all four tests turned green
Original comment by omar.sae...@gmail.com
on 18 May 2012 at 5:14
This issue was updated by revision r1765.
modified user creation part of the test
removed test user add interest since the method and view were deleted
Original comment by omar.sae...@gmail.com
on 18 May 2012 at 5:24
This issue was updated by revision r1770.
integration test turn green
Original comment by omar.sae...@gmail.com
on 18 May 2012 at 5:55
Removing the label UML-Ready after adding the comments..
Original comment by lydia.si...@gmail.com
on 19 May 2012 at 1:47
Check the comments on the UML in revision 1413..
Original comment by lydia.si...@gmail.com
on 19 May 2012 at 1:49
After talking to SAKR we reached these guidelines to make a proper
documentation.
First of all we will stick to that format of documentation for the whole
company.
=begin
Description:
Input:
Output:
Author:
=end
#Plus an inline documentation before each step u do inside that method
#for Views Please Document the Important parts of code & give an overview
about what this View does Exactly.
#For extra Help : Refer to my Actions in User Model. [after i commit for sure]
Original comment by kemoo...@gmail.com
on 19 May 2012 at 3:48
user_interests:
remove it not needed since there is an association between user and interests
for example:
user = current_user
all_interests = user.added_interests...check all the validations in the model
you are using
all_interests:
unneeded method remove it
toggle method:
indentation
int_toggle method:
better to rename it to be interest_toggle (more descriptive since int can be
interpreted wrongly by the reader)
DO NOT PASS ID TO ANY METHOD IN MODEL...pass the object itself
use associations do not insert directly in the db tables that represent
relations.
views:
indentation of a view
..divs must have all their contents inside them indented.
<div>
<p>...</p>
</div>
is_blocked (interest):
use associations
make the method true or false do not return int (ints are ambigoues in
representation to reader)
General Note for indentation in methods:
Note that: all lines should be less than 100 characters...no more and if a line
is longer inline it like that:
def example_story()
Story.create(:id => 1, :title =>
"jajajaja"...)
end
note that the next line is inline with the first or after it NOT before it with the def!!!
new_media_link ()
remove any commented lines
Original comment by rana.hel...@gmail.com
on 19 May 2012 at 7:43
My numbers mean the following :
1) Check that it compiles.
2)Check that it complies with the naming convention
3) Check that each assert has a description (message)
and there's a note section.
Unit/user_test :
test_add_delete_interest :
1) T
2) should_add_and_delete_interest
3) interest list should contain interest - interest list should not contain
that interest
Notes : use assert(not array.include? element) instead of having one item in
the list.
test_check_if_blocked:
1) T
2) should_block_interest
3) should return 1 - should return 2
Notes : Test invalid based on code reviewer comments (return boolean, not int)
test_get_all_interests:
1) T
2) should_get_all_interests
3) null
Notes : Test should be removed as it has no usage.
test_get_user_interests:
1) T
2) should_get_user_interests
3) both lists should be equal
Notes : user proper associations in your tests.
Functional/users_controller_test.rb
test_toggle_view:
1) T
2) should_display_toggle_view
3) toggle view should be gettable (not really sure of this one)
Integration/user_test.rb
test_route_to_toggle_interests:
1) T
2)
3)
Please note that this is the defensive part of your review (meaning that this
is the stuff that you've written) I've yet to check if there is need for an
offensive test review in your code.
Original comment by mina.ade...@gmail.com
on 19 May 2012 at 10:03
Original comment by kelbhaey
on 20 May 2012 at 1:10
This issue was updated by revision r2047.
followed documentation convention
Original comment by omar.sae...@gmail.com
on 20 May 2012 at 9:05
This issue was updated by revision r2103.
modified get method to add related stories
Original comment by omar.sae...@gmail.com
on 22 May 2012 at 3:31
This issue was updated by revision r2211.
fixed all interests method
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 11:54
This issue was updated by revision r2212.
removed some methods
fixed code
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 11:55
This issue was updated by revision r2213.
fixed according to code reviewer
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 11:56
but the view is going to be changed(all of it), just giving you enough time for
reviewing model and controller
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 11:58
This issue was updated by revision r2223.
deleted the old view added the new one
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 12:33
This issue was updated by revision r2224.
modified the view removed filter button if interest is blocked
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 12:46
stories controller :
indentation line 49
if line 50,52,54 (check note below)
user model:
569 id and id1?? names should be more descriptive
if 572, 579
582, 591 long unneeded space
586 remove commented lines
user_controller:
toggle method indentation everything inside method indented
toggle.html.erb:
line 5 too long
7,36 else in line with if
Note:
all ifs should be in line together and done like that:
if(condition)
then-part
else
else-part
end
Original comment by rana.hel...@gmail.com
on 23 May 2012 at 1:14
This issue was updated by revision r2231.
fixed implementation mistakes
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 1:35
This issue was updated by revision r2232.
fixed implementation mistakes
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 1:38
This issue was updated by revision r2233.
fixed implementation mistakes
Original comment by omar.sae...@gmail.com
on 23 May 2012 at 1:40
Original issue reported on code.google.com by
omar.sae...@gmail.com
on 10 May 2012 at 8:21