express examples #21

gireeshpunathil · 2020-07-17T13:49:26Z

Fixes: #10

bnb · 2020-07-17T16:28:04Z

Will take a more in-depth look soon here (probably Monday) but one tiny piece of feedback is that you're missing README.md for the added directories (see the countEntriesInDirectory CLI example and its parent directory, yargs as an reference for adding both an example and a category)and a GItHub Actions CI file (see .github/workflows/cli-yargs-countentriesindirectory.yml as a reference) for this specific project.

I do need to update the CONTRIBUTING.md to address the CI and testing tooling steps as it currently stands, my bad.

gireeshpunathil · 2020-07-18T06:19:46Z

@bnb - added a readme, aligning to the scope and purpose of this example. PTAL!

bnb · 2020-08-07T19:04:08Z

Retargeted to main, will check this as soon as I can - have been slammed with some other priorities recently ❤️

bnb · 2020-09-25T18:39:54Z

servers/express/README.md

+Gireesh Punathil <[email protected]>
+
+This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!


need to fix up the 'all contributors' section - I can do this <3

bnb · 2020-09-25T18:41:03Z

servers/express/app.js

+// An express example project to showcase few abstractions
+// General layout is:
+//   - source-in the modules that implement individual functions
+//   - create an express server app
+//   - initialize each modules and setup their routes
+//   - listen for clients


really nice commenting <3

servers/express/db.js

bnb · 2020-09-25T18:47:38Z

servers/express/db.js

+
+// client
+
+async function exec(q, r) {


if we could have more verbose / semantically meaningful variables than q and r that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T18:49:04Z

servers/express/db.js

+  const url = `mongodb://${HOST}:${PORT}`
+
+  // connect to the db server
+  MongoClient.connect(url, (e, c) => {


if we could have more verbose / semantically meaningful variables than e and and c that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T18:49:41Z

servers/express/defaults.js

+*/
+
+exports.setup = function(app) {
+  app.get('/', (q, r) => {


if we could have more verbose / semantically meaningful variables than q and r that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T18:52:32Z

servers/express/app.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

Since this repo is licensed as MIT, I'd prefer any license references to be MIT too to be consistent. Additionally, I'd generally prefer we don't have license headers in individual files and instead opt for LICENSE.md files

bnb · 2020-09-25T18:53:38Z

servers/express/db.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T18:54:01Z

servers/express/defaults.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T18:55:01Z

servers/express/index.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+*
+*     http://www.apache.org/licenses/LICENSE-2.0
+*
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T18:55:53Z

servers/express/package.json

+    "express-session": "^1.17.1",
+    "mongodb-memory-server": "^6.6.1",
+    "multer": "^1.4.2",
+    "tap": "^14.10.7"


tap should be a devDependency not a dependency.

bnb · 2020-09-25T18:56:51Z

servers/express/redirect.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T18:57:28Z

servers/express/redirect.js

+* limitations under the License.
+*/
+
+// A simple example that demonstrates


Suggested change

// A simple example that demonstrates

// An example that demonstrates

Just removing simple since that can be exclusionary to some folks ❤️

bnb · 2020-09-25T18:58:33Z

servers/express/redirect.js

+
+// A simple example that demonstrates
+// response redirect functionality
+// It redirects requests to /a, to /b


Could we use path names other than /a and /b? Perhaps something like /redirect and /landing or something?

bnb · 2020-09-25T18:59:40Z

servers/express/session.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:00:03Z

servers/express/test/db.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:00:18Z

servers/express/session.js

+exports.setup = function(app) {
+  app.use(session({ secret: 'random secret', cookie: { maxAge: 10000}}))
+
+  app.get('/session', (q, r) => {


if we could have more verbose / semantically meaningful variables than q and r that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:01:11Z

servers/express/test/db.js

+
+// test the /db route
+const url = 'http://localhost:12000/db'
+request(url, (e, r, b) => {


if we could have more verbose / semantically meaningful variables than e, r, and b that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:01:23Z

servers/express/test/defaults.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:02:07Z

servers/express/test/defaults.js

+
+// test the / route
+const url = 'http://localhost:12000'
+request(url, (e, r, b) => {


if we could have more verbose / semantically meaningful variables than e, r, and b that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:02:32Z

servers/express/test/forward.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:03:32Z

servers/express/test/forward.js

+
+// test the /a route
+const url = 'http://localhost:12000/a'
+request(url, (e, r, b) => {


if we could have more verbose / semantically meaningful variables than e, r, and b that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:03:37Z

servers/express/test/session.js

+
+// test the /session route
+const url = 'http://localhost:12000/session'
+request(url, (e, r, b) => {


if we could have more verbose / semantically meaningful variables than e, r, and b that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:03:40Z

servers/express/test/time.js

+
+// test the /time route
+const url = 'http://localhost:12000/time'
+request(url, (e, r, b) => {


if we could have more verbose / semantically meaningful variables than e, r, and b that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:03:54Z

servers/express/time.js

+// Return the current server time
+
+exports.setup = function(app) {
+  app.get('/time', (q, r) => {


if we could have more verbose / semantically meaningful variables than q and r that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:04:15Z

servers/express/upload.js

+  app.use('/upload', express.static('public'))
+
+  var diskstore = multer.diskStorage({
+    destination: function (q, f, cb) {


if we could have more verbose / semantically meaningful variables than q, f, and cb that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:04:23Z

servers/express/upload.js

+
+  var middleware  = multer({ storage: diskstore}).single('upload')
+
+  app.post('/upload', middleware, (q, r) => {


if we could have more verbose / semantically meaningful variables than q and r that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:05:32Z

servers/express/upload.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:05:39Z

servers/express/time.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:05:49Z

servers/express/test/time.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:05:53Z

servers/express/test/session.js

+/*
+* Licensed to the Apache Software Foundation (ASF) under one or more
+* contributor license agreements.  See the NOTICE file distributed with
+* this work for additional information regarding copyright ownership.
+* The ASF licenses this file to You under the Apache License, Version 2.0
+* (the "License"); you may not use this file except in compliance with
+* the License.  You may obtain a copy of the License at
+* 
+*     http://www.apache.org/licenses/LICENSE-2.0
+* 
+* Unless required by applicable law or agreed to in writing, software
+* distributed under the License is distributed on an "AS IS" BASIS,
+* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+* See the License for the specific language governing permissions and
+* limitations under the License.
+*/


Suggested change

/*

* Licensed to the Apache Software Foundation (ASF) under one or more

* contributor license agreements. See the NOTICE file distributed with

* this work for additional information regarding copyright ownership.

* The ASF licenses this file to You under the Apache License, Version 2.0

* (the "License"); you may not use this file except in compliance with

* the License. You may obtain a copy of the License at

*

* http://www.apache.org/licenses/LICENSE-2.0

*

* Unless required by applicable law or agreed to in writing, software

* distributed under the License is distributed on an "AS IS" BASIS,

* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

* See the License for the specific language governing permissions and

* limitations under the License.

*/

See first note on license statements

bnb · 2020-09-25T19:06:11Z

servers/express/test/defaults.js

+// test the / route
+const url = 'http://localhost:12000'
+request(url, (e, r, b) => {
+  if(e)


if we could have more verbose / semantically meaningful variables than e that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:06:32Z

servers/express/test/forward.js

+// test the /a route
+const url = 'http://localhost:12000/a'
+request(url, (e, r, b) => {
+  if(e)


if we could have more verbose / semantically meaningful variables than e that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:06:40Z

servers/express/test/db.js

+// test the /db route
+const url = 'http://localhost:12000/db'
+request(url, (e, r, b) => {
+  if(e)


if we could have more verbose / semantically meaningful variables than e that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:06:46Z

servers/express/test/session.js

+// test the /session route
+const url = 'http://localhost:12000/session'
+request(url, (e, r, b) => {
+  if(e)


if we could have more verbose / semantically meaningful variables than e that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:06:50Z

servers/express/test/time.js

+// test the /time route
+const url = 'http://localhost:12000/time'
+request(url, (e, r, b) => {
+  if(e)


if we could have more verbose / semantically meaningful variables than e that would be super helpful in giving more context to what we're doing with the variables.

bnb · 2020-09-25T19:08:51Z

servers/express/app.js

+
+  // listen for clients!
+  server = app.listen(12000, () => {
+    console.log('waiting for client connections!')


if we could console.log() the link with the port that would be super helpful in getting people going with the server

bnb · 2020-09-25T19:11:55Z

servers/express/session.js

+    } else {
+      q.session.views++
+    }
+    r.end(`thanks for visiting ${q.session.views}th time! (session expires in 10 seconds)`)


Might want to have a conditional here that changes th time! to st time! if the last digit in q.session.views is equal to 1.

bnb

Left a handful of comments. Most of them are roughly similar and can be addressed in batches.

Some additional feedback:

ideally we'd have this in a directory within the servers/express/ directory - so something like servers/express/basics or servers/express/buffet that gives an example of what's contained within.
- also change the name property in package.json to match this
the license should probably match the repo, which is MIT
testing locally, the file upload did not work
- error: Error: ENOENT: no such file or directory, open 'uploads/0c67fd383e608c955bdab9d9c2c8c261'

bnb · 2020-09-25T19:16:11Z

Additionally, we'd want to add a CI file for this project before merging 👍🏻

gireeshpunathil · 2020-09-29T06:41:09Z

@bnb - thanks! I guess I have addressed most of your comments. In addition,

ideally we'd have this in a directory within the servers/express/ directory - so something like servers/express/basics or servers/express/buffet that gives an example of what's contained within.
also change the name property in package.json to match this

moved the content into basics folder, and renamed the app ih the package.json, pls have a look and let me know if that is what you meant.

the license should probably match the repo, which is MIT

done, removed all license content. I guess the one in the repo vortex folder would suffice.

testing locally, the file upload did not work

The /upload assumes there is an uploads folder, and if it is missing, it throws.
There are two ways to solve this:

document this
pre-create a folder

wdyt?

Additionally, we'd want to add a CI file for this project before merging

any pointers? I have never made one.

thanks once again!

bnb · 2020-10-12T18:47:43Z

The /upload assumes there is an uploads folder, and if it is missing, it throws.
There are two ways to solve this:

document this

pre-create a folder

wdyt?

I'd say both if we can :P

bnb · 2020-10-12T18:50:38Z

On pointers for CI, I'd say start with the current CI workflow that we have - you basically just need to adapt it:

https://github.com/nodejs/examples/blob/main/.github/workflows/cli-yargs-countentriesindirectory.yaml

There's also the official reference and docs:

https://docs.github.com/en/free-pro-team@latest/actions/reference
https://docs.github.com/en/free-pro-team@latest/actions/learn-github-actions

I've also written a blog post on GitHub Actions CI that's relatively comprehensive and may be able to help answer any questions:

https://dev.to/bnb/an-unintentionally-comprehensive-introduction-to-github-actions-ci-blm

- make upload function more resilient - add a CI file - fix an issue with reirect function - remove an unwanted upload folder from the commit

gireeshpunathil · 2020-11-13T14:06:31Z

@bnb - I have pushed changes pertinent to out discussion, please take a look!

bnb · 2020-12-03T19:54:16Z

Seems that tap isn't getting installed which is failing the build. If you could figure out why, that'd be awesome :)

bnb · 2020-12-10T17:31:18Z

Looks like one of the tests is failing 😅

bnb changed the base branch from master to main August 7, 2020 19:02

bnb reviewed Sep 25, 2020

View reviewed changes

servers/express/db.js Outdated Show resolved Hide resolved

bnb reviewed Sep 25, 2020

View reviewed changes

bnb suggested changes Sep 25, 2020

View reviewed changes

express: initial commit

6ef5b38

gireeshpunathil force-pushed the express branch from 90a42ff to 6ef5b38 Compare September 29, 2020 06:33

gireeshpunathil added 2 commits November 13, 2020 19:05

fixup: address review comments

6e25761

- make upload function more resilient - add a CI file - fix an issue with reirect function - remove an unwanted upload folder from the commit

fixup: fix linting errors

fdee6e8

fixup: install tap as a dev dependency

4da5211

crandmck mentioned this pull request Mar 5, 2024

express example programs in Node.js example expressjs/discussions#136

Open

		Gireesh Punathil <[email protected]>

		This project follows the [all-contributors](https://github.com/all-contributors/all-contributors) specification. Contributions of any kind welcome!

	// A simple example that demonstrates
	// An example that demonstrates


		var middleware = multer({ storage: diskstore}).single('upload')

		app.post('/upload', middleware, (q, r) => {

express examples #21

Are you sure you want to change the base?

express examples #21

Conversation

gireeshpunathil commented Jul 17, 2020

bnb commented Jul 17, 2020

gireeshpunathil commented Jul 18, 2020

bnb commented Aug 7, 2020

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

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

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

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

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnb left a comment • edited Loading

Choose a reason for hiding this comment

bnb commented Sep 25, 2020

gireeshpunathil commented Sep 29, 2020

bnb commented Oct 12, 2020

bnb commented Oct 12, 2020

gireeshpunathil commented Nov 13, 2020

bnb commented Dec 3, 2020

bnb commented Dec 10, 2020

bnb left a comment •

edited

Loading