Refactored the project #21

ayuhsya · 2017-03-11T12:29:41Z

Moved all images to /resources
Moved all scripts to /scripts
Moved all data to /data
Moved css to /styles
fuse.min.js now loads from CDN instead of local storage

athityakumar

Hey @ayuhsya ! Thanks for opening this PR! 😄

Have a look at the changes suggested. Also, typically, the sitemap looks like this -

resources/
     images/
     js/ or scripts/
     css/ or styles/

Change the html files accordingly, to fit this the above sitemap.

athityakumar · 2017-03-11T12:40:25Z

add_node.rb

@@ -42,7 +42,7 @@ def default today
  ARGV.clear
 end

-obj = JSON.parse(File.read("data.json"))
+obj = JSON.parse(File.read("./data/data.json"))


Just obj = JSON.parse(File.read("data/data.json")) will do.

athityakumar · 2017-03-11T12:41:03Z

add_node.rb

@@ -117,10 +117,10 @@ def default today

 if not testing
  puts "#{obj.length} papers now."
-  File.delete("data.json")
+  File.delete("./data/data.json")


Just File.delete("data/data.json") will do.

athityakumar · 2017-03-11T12:41:51Z

add_node.rb

  if pretty
-    File.open("data.json", "w") { |file| file.write(JSON.pretty_generate(obj)) }
+    File.open("./data/data.json", "w") { |file| file.write(JSON.pretty_generate(obj)) }


Same issue as above comment.

athityakumar · 2017-03-11T12:42:06Z

add_node.rb

  else
-    File.open("data.json", "w") { |file| file.write(JSON.generate(obj)) }
+    File.open("./data/data.json", "w") { |file| file.write(JSON.generate(obj)) }


Same issue as above comment.

athityakumar · 2017-03-11T13:05:51Z

@ayuhsya - No need to move the data/ folder into resources/. Just make this change, and I'll merge it, if no other changes are required.

athityakumar · 2017-03-11T13:16:26Z

index.html

@@ -1,10 +1,11 @@
 <html>
    <head>
        <script src="https://code.jquery.com/jquery-2.1.4.min.js"></script>
-        <script src="string_score.min.js"></script>
+        <script src="./resources/scripts/string_score.min.js"></script>


Change all the ./resources/ in this file to resources/. And we're good to go. 👍

athityakumar · 2017-03-11T13:17:10Z

index.html


-             $.getJSON("data.json")
+             $.getJSON("./data/data.json")


Change this to $.getJSON("data/data.json")

athityakumar · 2017-03-11T13:28:15Z

resources/scripts/worker.js

@@ -1,5 +1,3 @@
-importScripts('fuse.min.js');


I don't think that the new Fuse() function in this file will work without import fuse.min.js file. We do need it.

@athityakumar How can I test it?

For usual JS testing, you can check out the Console on your browser (Inspect > Console option appears). However, testing for worker js files would have to be done on server - you can push to your fork, publish that branch in your fork as a gh-pages website. Then, have a look at ayuhsya.github.io/mfqp/ and check the Console of that website.

@ayuhsya - Or you can just restore the fuse.min.js file back, and I'll merge this. Maybe we can work on loading fuse.min.js from CDN on another PR. 😄

amrav · 2017-03-11T13:47:10Z

Hmm, why can't you test js workers in the browser locally?

…

On Sat, 11 Mar 2017 at 13:43, Athitya Kumar ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In resources/scripts/worker.js <#21 (comment)>: > @@ -1,5 +1,3 @@ -importScripts('fuse.min.js'); For usual JS testing, you can check out the Console on your browser (Inspect > Console option appears). However, testing for worker js files would have to be done on server - you can push to your fork, publish that branch in your fork as a gh-pages website. Then, have a look at ayuhsya.github.io/mfqp/ and check the Console of that website. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#21 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABIZ_yrXeB2h5QB8gq1tKXt4tpuzi1qoks5rkqTsgaJpZM4MaMOL> .

icyflame · 2017-03-11T19:14:36Z

@athityakumar @ayuhsya I think you guys don't know about SimpleHTTPServer 🙂

You can test everything (including JavaScript) by running:

$ python -m SimpleHTTPServer

from the mfqp/ directory and then visiting localhost:8000 on your browser.

@amrav This should fix their problem.

ayuhsya · 2017-03-11T20:22:44Z

@icyflame I was using SimpleHTTPServer itself to test the HTML all this while, there's some Apache thing to create local server too, but I find it inconvenient to use.

I just can't find any usage of the file worker.js in the project that's why I asked how to test it. I'll just restore the fuse script for this PR.

amrav · 2017-03-11T22:11:22Z

You can also use node's http-server, which is async and gives slightly nicer output IMO. (npm install -g http-server) I think worker.js is unused. This whole thing was hacked together in a night or two, so code quality went out of the window - please reactor/rewrite liberally :)

…

On Sat, 11 Mar 2017 at 20:22, Ayushya Anand ***@***.***> wrote: @icyflame <https://github.com/icyflame> I was using SimpleHTTPServer itself to test the HTML all this while, there's some Apache thing to create local server too, but I find it inconvenient to use. I just can't find any usage of the file worker.js in the project that's why I asked how to test it. I'll just restore the fuse script for this PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#21 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABIZ_7ihYRwTvan4kxuK02aqc9_sLIieks5rkwKUgaJpZM4MaMOL> .

icyflame · 2017-03-12T21:13:49Z

@athityakumar refactor looks good. repo looks pretty now. Thanks! 😄

refactored the project

123c0e1

athityakumar suggested changes Mar 11, 2017

View reviewed changes

ayuhsya added 2 commits March 11, 2017 18:24

moved all folders under resources

b1f45f5

updated add_node.rb

b25bfe9

removed data from resources

abff0b0

athityakumar reviewed Mar 11, 2017

View reviewed changes

index.html Outdated

$.getJSON("data.json")

$.getJSON("./data/data.json")

Copy link

Member

athityakumar Mar 11, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change this to $.getJSON("data/data.json")

updated href links

0580bbc

athityakumar reviewed Mar 11, 2017

View reviewed changes

replaced fuse.min.js

cfa0c35

athityakumar approved these changes Mar 11, 2017

View reviewed changes

Merge branch 'gh-pages' into ref

84b8cf8

athityakumar merged commit bdeab25 into metakgp:gh-pages Mar 11, 2017

ayuhsya deleted the ref branch March 11, 2017 21:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactored the project #21

Refactored the project #21

ayuhsya commented Mar 11, 2017 •

edited

Loading

athityakumar left a comment •

edited

Loading

athityakumar Mar 11, 2017

athityakumar Mar 11, 2017

athityakumar Mar 11, 2017

athityakumar Mar 11, 2017

athityakumar commented Mar 11, 2017

athityakumar Mar 11, 2017

athityakumar Mar 11, 2017

athityakumar Mar 11, 2017

ayuhsya Mar 11, 2017

athityakumar Mar 11, 2017

athityakumar Mar 11, 2017 •

edited

Loading

amrav commented Mar 11, 2017 via email

icyflame commented Mar 11, 2017

ayuhsya commented Mar 11, 2017

amrav commented Mar 11, 2017 via email

icyflame commented Mar 12, 2017

Refactored the project #21

Refactored the project #21

Conversation

ayuhsya commented Mar 11, 2017 • edited Loading

athityakumar left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

athityakumar commented Mar 11, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

athityakumar Mar 11, 2017 • edited Loading

Choose a reason for hiding this comment

amrav commented Mar 11, 2017 via email

icyflame commented Mar 11, 2017

ayuhsya commented Mar 11, 2017

amrav commented Mar 11, 2017 via email

icyflame commented Mar 12, 2017

ayuhsya commented Mar 11, 2017 •

edited

Loading

athityakumar left a comment •

edited

Loading

athityakumar Mar 11, 2017 •

edited

Loading