WIP: Refactoring #3
Conversation
Enforce local building of code--for now. Prevent accidently over sharing credentials.
This provides for bootstrapping a hosting directory with a minimal discovery HTML document. It is copied on the first run of the server, but not on subsequent runs to avoid overwriting changes. Re-bootstrapping simply requires moving the `index.html` from the `rootPath` directory.
rootPath is now...root. Other paths are all expected to be served from inside of rootPath (relatively).
Avoids double // issues
Move to src/server.js
This is a bit heavier refactoring than I like to do in one go. getSerialization() function signature now includes passing in rdfaTypes. Small smell...but we can fix that later.
Mostly for future use by other projects
|
👍 for merging this @csarven, very much needed. For the server, I hope we end up with different express middleware that can be used separately to run inboxes as a library. |
|
I'm also +1 to this but IIRC the changes broke https://github.com/csarven/ldn-tests/ . I'd prefer to merge once ldn-tests factors in the changes in mayktso. [mayktso was initially created for ldn-tests and then pulled out.. I don't want to track two different mayktsos in production.] |
|
Of course, but what is still needed for that? And https://github.com/csarven/ldn-tests/ should probably use this repo as a dependency/library? |
|
Besides bring this PR up to date with current master? .. Off the top of my head, we need to be careful with the URL and file paths. ldn-tests already includes maytkso in its package as a dependency. |
Maybe it's better to redo the refactoring on the current master with this PR as inspiration. |
|
@csarven the conflicts don't seem that bad, actually. You can use the "Resolve conflicts" button here to preview (and manage them too, actually). I'm not familiar enough with ldn-tests to know what I've broken (or not) about it. You mentioned URL and file paths. Are there enough tests in that package that could confirm (or deny) any refactoring success? I'm happy to take another crack at this. 😃 |
| @@ -0,0 +1,13 @@ | |||
| module.exports = function(req, res, next) { | |||
There was a problem hiding this comment.
@BigBlueHat can't this be replaced with https://www.npmjs.com/package/cors?
There was a problem hiding this comment.
Probably, but I'd recommend doing that as a separate PR. The goal of this PR was to simply refactor the existing code--not introduce any new dependencies, etc.
Once this PR is in, then yes, it would be great to reconsider adding dependencies.
|
@BigBlueHat Any plans to continu this? |
This one goes on top of
host-out-of-www(once/if that's merged).The objective is smaller bits of more easily testable/understandable/reusable code.
The goal is to end up with three chunks of code: server, client, and cli. The cli is the command line interface to client. The other two do what they say on the tin. 😁