Lua Toolbox merge #86

Etiene · 2016-03-17T18:52:25Z

A brand new dump of LuaToolbox's db will be needed (cc @catwell)

For transferring endorsements to Followings I added a new tab on the user settings explaining the situation with a link for the transfer. I have an issue once the transfer is done though. I'm rendering the same view from a different action and this results on the url changing when you get the success message. I don't particularly like that but I didn't know how to do otherwise and this might need a touch-up. Not a huge deal, though.

When importing current labels I was not exactly sure how to access Lapis' model module in a simple script. So for the moment the functions are in a helper class and I tested and ran them using the specs.

I also added the labels to the front page, but I'm not sure if it's pretty enough.

For the moment only module owners can add or remove labels from a module. There's no way to add new labels yet, only to select from existing labels.

Screenshots of my local system:

|--> Owner's view: Add label link is on owner's menu for the moment

catwell · 2016-03-17T19:58:52Z

This is great. I still miss one of the main features from Toolbox though: the ability to see the list of modules endorsed / followed by someone and a list of people endorsing / following a module. If this isn't available then followings cannot replace endorsements.

The first one could be added below the list of modules authored by a user on their user page and the second one could be accessible by clicking here:

Regarding DB dumps, if I give you a dump and at a later time another one, can you apply the changes? I am starting to think about exactly when I should shut Toolbox down.

Etiene · 2016-03-17T20:01:55Z

the ability to see the list of modules endorsed / followed by someone and a list of people endorsing

oops! :) coming in soon :)

catwell · 2016-03-17T20:03:44Z

Awesome!

I am still wondering if "follows" and "endorsements" should be the same thing. On GitHub they are not (Follow would be Watch and Endorse would be Star). What do you think?

Etiene · 2016-03-17T20:07:54Z

I had this discussion with @leafo , the issue is that maybe this makes for too many verbs and features for the users to catch up for now and the current following feature doesn't do much for the moment, he suggested merging them up and I thought it was a good idea. @hishamhm suggested maybe we should rename the followings to something simpler, such as stars, so user engagement can come easier.

catwell · 2016-03-17T20:10:44Z

OK, it makes sense.

Regarding missing features, I also think that:

Follow count should be visible in module lists (next to downloads)
There should be a way to sort those lists by follows (and probably downloads too)

Regarding the transition, here is how I see it:

Once this is ready, we decide on a date to shutdown Lua Toolbox.
I will send an email to all Toolbox users, telling them that it is shutting down.
- I will explain how it will be replaced by LuaRocks (including the import feature).
- I will tell users to create accounts there if they haven't already.
On the date we decided I will replace the Lua Toolbox website by a static page explaining this.
I will generate a final DB dump that I will give you.

How does that sound?

hishamhm · 2016-03-17T20:46:09Z

Yes, my argument was that "follow" makes me think "no, I don't want to
receive email notifications", while "stars" are vague enough so that in
practice it has a community-defined meaning (see the old Twitter stars).

I also suggested that, if possible, it would be nice to import Github stars
from a project and add them up with the "local stars". This would help
populating project stars a lot.
On Mar 17, 2016 17:11, "Etiene Dalcol" notifications@github.com wrote:

I had this discussion with @leafo https://github.com/leafo , the issue
is that maybe this makes for too many verbs and features for the users to
catch up for now and the current following feature doesn't do much for the
moment, he suggested merging them up and I thought it was a good idea.
@hishamhm https://github.com/hishamhm suggested maybe we should rename
the followings to something simpler, such as stars, so user engagement can
come easier.

—
You are receiving this because you were mentioned.
Reply to this email directly or view it on GitHub
#86 (comment)

Etiene · 2016-03-17T23:51:37Z

@catwell sounds perfect to me!

leafo · 2016-03-18T07:46:48Z

applications/modules.moon

+
+  [modules_label: "/label/modules/:label"]: capture_errors_404 =>
+    label = assert_error Labels\find(name: @params.label), "Invalid label"
+    return unless label


assert error jumps out of the function, so this return is unnecessary

leafo · 2016-03-18T08:19:16Z

spec/helpers/toolbox_spec.moon

+import
+  modules
+  labels
+  from require "secrets.toolbox"


I don't think specs should depend on the secrets, since anyone who checks out the code will not be able to run the specs successfully. Maybe we can have a fake dump stored elsewhere

I agree! I only added it there because I'm not sure how else to run the function I created from within helpers to import the labels from the lua dump to the sql. Should we make a separate script to do that, and then we can remove this from here or add the fake dump?

leafo · 2016-03-18T08:37:53Z

Thanks for the patch, I did a quick code review. Tell me if you have any questions. Didn't get a chance to run the code and check out the UI yet.

I named the functionality follow because to me that says you'd like to follow the changes/updates of the module. I planned to have some sort of feature where it would give you a digest page on the site of modules you follow that got updated, and an optional email when new versions of modules you follow are released.

I think having github stars displayed could be a great way for someone browsing modules to identify what's used, but not sure about he complexities in keeping in synchronized.

leafo · 2016-03-18T08:40:57Z

applications/toolbox.moon

+		      		}
+	      			@transfer_count = @transfer_count+1
+
+      render: "user_settings.import_toolbox"


Once you change this action to post, you should turn this into a redirect to go to the page that renders this view. You can pass transfer count as a query param

I decided to move this into a helper function that returns the count

leafo · 2016-03-21T22:56:13Z

Are you planning on making these changes @Etiene? I know your time is limited for this project, no need to rush, just curious.

Etiene · 2016-03-21T23:10:13Z

the time for the project is up, had to shift to report writing, but I will make these changes! :)

commit e5ec6ff Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 23:03:50 2016 +0200 Refactor commit a69197b Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 22:15:07 2016 +0200 Refactor commit 368cd1b Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 22:04:16 2016 +0200 Refactor commit 5240e5f Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 21:59:43 2016 +0200 Refactor: User post on import endorsements commit b220a1e Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 01:10:01 2016 +0200 Refactor commit c6187dd Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 00:39:53 2016 +0200 Fix indentation commit 70f3c0e Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 00:34:56 2016 +0200 Change style on url redirect on remove label commit f7ae6d7 Author: Etiene Dalcol <dalcol@etiene.net> Date: Fri May 6 00:15:42 2016 +0200 Use list comprehension on module label list commit c384ebd Author: Etiene Dalcol <dalcol@etiene.net> Date: Thu May 5 20:09:38 2016 +0200 Remove unnecessary return

Etiene · 2016-05-06T21:15:24Z

Sorry for the delay! Finishing masters, moving to a different country and stuff!

@leafo I've made a refactor following most of your suggestions. I'll need help with some of them, though, I'll reply to those one by one. Could you take another look now?

@catwell I'll start working on your suggestions now

leafo · 2016-07-06T03:38:53Z

widgets/module_header.moon

+        action: @url_for(@module_endorsing and "unendorse_module" or"endorse_module", module_id: @module.id)
+        method: "post"
+      }, ->
+        @csrf_input


this should be method call

leafo · 2016-09-27T05:00:17Z

Alright! Sorry for the huge delay. I've merged in your changes. I hope you don't mind but I ended up refactoring things a bit to simplify the model structure. Labels are stored right on the module with an postgres array type. I also moved the label stuff into a new application, and added tests.

I've hidden the endorsement -> follows stuff at the moment. Once I get the data dump I can test running it, then add it back.

Thanks for your help getting this stuff done, sorry for the slow turnaround time.

@catwell could you email me that dump when you get a chance?

Thanks

catwell · 2016-09-27T06:27:03Z

@leafo email sent.

Etiene · 2016-09-27T20:18:53Z

No, dont worry, thank you! I'm sorry it took me ages to reply as well and I didn't have the time to go through all your suggestions! I'm really glad this is happening! 👯

catwell · 2016-09-29T10:13:39Z

I have seen that this is merged, awesome!

All that's missing for me now is:

follows count on module summaries;
ability to sort module lists by endorsements or by follows.

I will set Lua Toolbox read-only and point users to LuaRocks as soon as I find the time. (EDIT: done)

Etiene added 10 commits March 10, 2016 16:14

Add endorsements transfer to followings

ed3358c

Merge branch 'master' of https://github.com/Etiene/luarocks-site

e1f102d

Add message for endorsement transfer success

cc247be

Replace message on import endorsement success

a4a93e4

Add functions to import current labels from toolbox

774eb01

Add view modules in label and labels module

f8693e1

Add remove label option

bf78579

Add label to module option

43a84bb

View labels on index

bbc443f

Sort labels alphabetically when adding

4c7a67a

leafo reviewed Mar 18, 2016
View reviewed changes

leafo reviewed Jul 6, 2016
View reviewed changes

leafo merged commit 4140bce into luarocks:master Sep 27, 2016

catwell mentioned this pull request Dec 22, 2016

Adding Lua Toolbox to MoonRocks catwell/lua-toolbox#18

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Lua Toolbox merge #86

Lua Toolbox merge #86

Etiene commented Mar 17, 2016 •

edited

Loading

catwell commented Mar 17, 2016

Etiene commented Mar 17, 2016

catwell commented Mar 17, 2016

Etiene commented Mar 17, 2016

catwell commented Mar 17, 2016

hishamhm commented Mar 17, 2016

Etiene commented Mar 17, 2016

leafo Mar 18, 2016

leafo Mar 18, 2016

Etiene May 6, 2016

leafo commented Mar 18, 2016

leafo Mar 18, 2016

Etiene May 6, 2016

leafo commented Mar 21, 2016

Etiene commented Mar 21, 2016

Etiene commented May 6, 2016 •

edited

Loading

leafo Jul 6, 2016

leafo commented Sep 27, 2016

catwell commented Sep 27, 2016

Etiene commented Sep 27, 2016

catwell commented Sep 29, 2016 •

edited

Loading

Lua Toolbox merge #86

Lua Toolbox merge #86

Conversation

Etiene commented Mar 17, 2016 • edited Loading

catwell commented Mar 17, 2016

Etiene commented Mar 17, 2016

catwell commented Mar 17, 2016

Etiene commented Mar 17, 2016

catwell commented Mar 17, 2016

hishamhm commented Mar 17, 2016

Etiene commented Mar 17, 2016

leafo Mar 18, 2016

Choose a reason for hiding this comment

leafo Mar 18, 2016

Choose a reason for hiding this comment

Etiene May 6, 2016

Choose a reason for hiding this comment

leafo commented Mar 18, 2016

leafo Mar 18, 2016

Choose a reason for hiding this comment

Etiene May 6, 2016

Choose a reason for hiding this comment

leafo commented Mar 21, 2016

Etiene commented Mar 21, 2016

Etiene commented May 6, 2016 • edited Loading

leafo Jul 6, 2016

Choose a reason for hiding this comment

leafo commented Sep 27, 2016

catwell commented Sep 27, 2016

Etiene commented Sep 27, 2016

catwell commented Sep 29, 2016 • edited Loading

Etiene commented Mar 17, 2016 •

edited

Loading

Etiene commented May 6, 2016 •

edited

Loading

catwell commented Sep 29, 2016 •

edited

Loading