Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introducing a View layer #143

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

Conversation

mulderp
Copy link
Contributor

@mulderp mulderp commented Jul 20, 2013

Extracting the views into their own layer (e.g. with ERB), might be nice to integrate custom layouts, and as preparation to improve on the the client-side interaction part too.

The presented approach still depends on the HTML generator, and it takes some work to use ERB everywhere. On the ohter hand, the generator looks like making a nice Gem, that could be included in the project (maybe Hg would be a nice name)

This PR is not yet fully finished, but maybe it is already good for some discussion.

Thanks!

@mulderp
Copy link
Contributor Author

mulderp commented Jul 24, 2013

the '/about' page seems still broken, need to fix this

This was referenced Jul 24, 2013
@mulderp
Copy link
Contributor Author

mulderp commented Jul 25, 2013

another bug is replying to comments and editing news

@fcambus
Copy link
Collaborator

fcambus commented Aug 2, 2013

I don't think this has chances to get merged into Lamer News.

Template usage has been debated a few times already, notably here : http://www.lamernews.com/news/253

I agree with @antirez position about templates regarding Lamer News use case :)

@mulderp
Copy link
Contributor Author

mulderp commented Aug 3, 2013

Thanks for the feedback and the link! Yes, templates would impact the architecture quite a lot.

it would be great to see a oneliner (or small script), to compare the impact of templates on the page load. I remember I saw something like this: https://github.com/wintermeyer/snappy_webshop/blob/master/lib/speed-test.rb - but for sure there is a simpler test.

Actually, I am also reflecting whether I could move the templates out to the client-side completely with support of a client-side framework such as Backbone.js. Another small learning from the discussion might be to encapsulate the view routes in a kind of module that could be swapped as desired.

Well, just in case someone wants to share ideas, give me a ping

@mulderp
Copy link
Contributor Author

mulderp commented Aug 5, 2013

Just for the archive, on my small Macbook Air, I get:

  • HTML generator:
    Transaction rate: 29.47 trans/sec
    Longest transaction: 0.07s
  • View templates
    Transaction rate: 28.90 trans/sec
    Longest transaction: 0.12s

So, it seems roughly to be the same throughput, with a slightly higher variance on the template approach.

@mulderp
Copy link
Contributor Author

mulderp commented Aug 9, 2013

as a reminder (partly for myself), the Telescope UI looks interesting too: https://github.com/SachaG/Telescope

@fcambus
Copy link
Collaborator

fcambus commented Aug 11, 2013

About template usage, it's not a lot slower in this case because news items are still generated with the HTML generator. I believe the throughput would change dramatically when adding partials to render each news item, especially as by default Lamer News is configured to display 100 items on top and latest pages.

For the record, Hacker News use the same approach (HTML generator), here is their implementation : https://github.com/wting/hackernews/blob/master/html.arc :D

@kawa-
Copy link

kawa- commented May 30, 2015

@mulderp Thanks! I like the view layer by you.

@mulderp
Copy link
Contributor Author

mulderp commented May 30, 2015

@kawa- thanks, at the end, the only use of my refactoring was some views for an experimental port of Lamernews to Nodejs https://github.com/mulderp/echojs

@kawa-
Copy link

kawa- commented May 30, 2015

@mulderp 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants