rebased
Fixes #1129
You should use something like unwatch in place of i. i doesn't convey anything.
unwatch
i
here!
I don;t know if its a good idea to make other people see who is watching which all projects.
I actually have this should we have a panel on the side where list of watched projects are mentioned , do we need a different page all together ?
I can see the to-do list of pingou here so, i think it should be fine
It will become difficult when the number of projects becomes bigger.
Yeah, i would lean against adding a new page for this -- adding them to the user's home page is probably the way to go.
That said, the homepage could do with some rework where we (perhaps) move these all into seperate pages (my repos, forks, watched repos, groups, etc) but currently, these are all on the user home page (i.e. pagure.io when logged in), so we should keep the watched list there too for now.
I am not done yet. So, don't review atm
open to review now.
This is how it looks: http://img.susepaste.org/24954063
1 new commit added
ATM: http://img.susepaste.org/52738170
Hey good work there , my take would be that watchlist is on the side bellow groups since there is a lot of place you can utilize there!
@ryanlerch you too think that it should be below the groups?
is this one better? http://img.susepaste.org/5851306
I like, looks nice! :)
:thumbsup: for this !
one problem that i see myself is that i have used set() at one point because of that the watch list will be arranged in a different way about every time :stuck_out_tongue: (i will think of something different if anybody doesn't like this)
set()
I don't think that as a big issue , but you can always do sorted(<set>) to achieve order.
sorted(<set>)
pep8 will likely ask for a single line here and above the function as well
This could use a test in tests/pagure_lib.py
tests/pagure_lib.py
If you want to sort that list :thumbsup: for me
This is just looking at the code, I'll test it locally for the UI
@ryanlerch if you could also have a look at it from an UI point of view, that would be nice :)
this is how it looks: http://img.susepaste.org/69070857
:thumbsup: from me from the UI perspective
I have replaced None by [] here. I thought: If this line was ever hit, it would have resulted in a crash because in the template we are using | count.
None
[]
| count
Jinja is pretty error proof, so I'm not sure it would have mattered but good idea nonetheless :)
If this line was ever hit, it would have resulted in a crash because in the template we are using | count.
This line makes me wonder: are you covering this in your unit-tests? If not they, we should fix it :)
@pingou i used __get_user which raises exception when user is not found. I have changed it to search_user and added the unknown user check in the tests as well. probably, shouldn't have used __get_user. :/
__get_user
search_user
Alright, let's rebase and merge :)
Pull-Request has been merged by pingou