From kde-active Fri May 31 23:01:34 2013 From: "Aaron J. Seigo" Date: Fri, 31 May 2013 23:01:34 +0000 To: kde-active Subject: Re: Review Request 110752: Bodega-server: Add documentation support to bodega-server. Also add markd Message-Id: <20130531230134.23367.77061 () vidsolbach ! de> X-MARC-Message: https://marc.info/?l=kde-active&m=137004131508557 MIME-Version: 1 Content-Type: multipart/mixed; boundary="--===============4743860081115362754==" --===============4743860081115362754== Content-Type: multipart/alternative; boundary="===============7119491844792470914==" --===============7119491844792470914== Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110752/#review33542 ----------------------------------------------------------- i don't think it makes much sense to include the README file, since that is required to get the server started. which makes it a bit of a chicken-and-egg problem ;) i also don't see much value in having the README available there. what might be more interesting is a license file in the docs. also, this needs a test before it can go in. test should ensure that /api/ works, that requesting a non-existent file fails and requesting and existing file returns the actual file contents (the test can load it from disk to compare) server/routes.js this means we can't have subdirs in the doc/ directory. perhaps better would be to call path.normalize .. sth like: app.get('/api/(.*)', function(req, res) { var filename = path.normalize(req.params[0]); this allows subdirs, and protects against things like '..' causing problems. server/routes.js this should be an async read with the callback sending the file contents. also, if the file does not exist the 404 page should be sent. - Aaron J. Seigo On May 31, 2013, 5:59 p.m., Antonis Tsiapaliokas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/110752/ > ----------------------------------------------------------- > > (Updated May 31, 2013, 5:59 p.m.) > > > Review request for Bodega. > > > Description > ------- > > Hello > > with that patch i am adding the following support to the bodega-server: > > 1) server/doc is the place which all of our documentaion lives. > 2) The docs are accessible from /api/filename.E.x. for docs/README.markdown => api/README > > If you all agree with that patch, i will push it to the branch which i am referring above(kokeroulis/markdown/documentation) > > P.S. the two files are from bodega-server and bodega-client > > > Diffs > ----- > > server/doc/index.markdown PRE-CREATION > server/doc/json_api.markdown PRE-CREATION > server/package.json 09f2527 > server/routes.js 95883a9 > > Diff: http://git.reviewboard.kde.org/r/110752/diff/ > > > Testing > ------- > > > Thanks, > > Antonis Tsiapaliokas > > --===============7119491844792470914== Content-Type: text/html; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit

This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/110752/

i don't think it makes much sense to include the README file, since that is required to get the server started. which makes it a bit of a chicken-and-egg problem ;) i also don't see much value in having the README available there. what might be more interesting is a license file in the docs.

also, this needs a test before it can go in. test should ensure that /api/ works, that requesting a non-existent file fails and requesting and existing file returns the actual file contents (the test can load it from disk to compare)

server/routes.js (Diff revision 1)
function serverPath(path)
		366	var filename = path.basename(req.url);

this means we can't have subdirs in the doc/ directory. perhaps better would be to call path.normalize .. sth like:

app.get('/api/(.*)', function(req, res) {
    var filename = path.normalize(req.params[0]);

this allows subdirs, and protects against things like '..' causing problems.

server/routes.js (Diff revision 1)
function serverPath(path)
		376	var filenameData = fs.readFileSync(filePath, 'utf8');

this should be an async read with the callback sending the file contents.

also, if the file does not exist the 404 page should be sent.

- Aaron J.

On May 31st, 2013, 5:59 p.m. UTC, Antonis Tsiapaliokas wrote:

Review request for Bodega.

By Antonis Tsiapaliokas.

Updated May 31, 2013, 5:59 p.m.

Description

Hello

with that patch i am adding the following support to the bodega-server:

1) server/doc is the place which all of our documentaion lives.
2) The docs are accessible from /api/filename.E.x. for docs/README.markdown => api/README

If you all agree with that patch, i will push it to the branch which i am referring above(kokeroulis/markdown/documentation)

P.S. the two files are from bodega-server and bodega-client

Diffs

server/doc/index.markdown (PRE-CREATION)
server/doc/json_api.markdown (PRE-CREATION)
server/package.json (09f2527)
server/routes.js (95883a9)

--===============7119491844792470914==-- --===============4743860081115362754== Content-Type: text/plain; charset="us-ascii" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Content-Disposition: inline _______________________________________________ Active mailing list Active@kde.org https://mail.kde.org/mailman/listinfo/active --===============4743860081115362754==--