Proposed changes for ThirdRail compatibility #171
Conversation
Also, can't use ||= without setting default value from Volt.root's internal @root ||=
Also, kill unneeded VOLT_PATH constant
Both seem like improvements though, and calling to_s lets us use symbols instead of strings if/when we get server side rendering to work
Server.index_files still feels like a hack/ bad idea, but can discuss in PR
third-rail is working for me without them in any case, can discuss if they make sense anyway
We appear to be able to generate the files from within third-rail, and if not a better/cleaner interface than this would be preferable anyway
This reverts commit eee1f53.
| @@ -34,7 +34,7 @@ def connect! | |||
| var wsProto = 'ws'; | |||
| } | |||
|
|
|||
| this.socket = new WebSocket(wsProto + '://' + document.location.host + '/socket'); | |||
| this.socket = new WebSocket(wsProto + '://' + document.location.host + '/volt/socket'); | |||
There was a problem hiding this comment.
Not 100% sure this is needed, but seems like it would be with others.
|
Test failures are from namespace changes/prepending /volt to paths, which is one of the things I want to make sure you're OK with, but I'll fix specs for this assuming its OK |
|
Sorry, today was swamped, I'll check it out tomorrow. Thanks! |
|
@ryanstout this keeps getting merge conflicts, I've fixed two, but don't want to bother fixing more until you've had a look at the general shape of changes and how close/far you are from being comfortable merging them. Maybe @catmando or @pencilcheck will weigh in or want additional changes too, don't know if you prefer to get it all in in one PR, or happy to have several. |
|
@bglusman sorry I've been so slow on this one. Can you move all of the method_defined checks into their own PR? I would rather not namespace all of volt by default. But I'm planning to add app name-spacing in by default so it can be embedded in another rack app anyway. So maybe we could pair sometime soon and knock that out? |
|
@bglusman did you see my last comment? I've got time if you want to try and get everything worked out and integrated. |
|
Thanks Ryan, I saw and I've discussed a little in third rail room, think I On Sunday, May 10, 2015, Ryan Stout notifications@github.com wrote:
-Brian Note: Sent from handheld, please excuse brevity, typos, transcription |
|
@bglusman no problem. Yea, smaller PR's would be good. I've got a branch where I'm working on adding namespacing so you can just embed volt as a rack app. (Also trying to get rid of the $page global) |
|
Awesome, you have a link to namespaces branch? Maybe I should branch off of On Sunday, May 10, 2015, Ryan Stout notifications@github.com wrote:
-Brian Note: Sent from handheld, please excuse brevity, typos, transcription |
|
@bglusman its pretty early on that branch. Some of it has gotten merged recently. |
|
Closing in favor of #210 |
Hey @ryanstout thanks for your help today, though turned out not to be that and only to be my stupidity as usual.
Hopefully these changes are more or less acceptable as is, but let us know if you want any tweaks made, or if there's anything we need to rethink/rework significantly. Hopefully once this is merged in we can consider a third rail alpha release!
Thanks to @catmando and @pencilcheck, if either of you has any thoughts or suggestions speak up.