Summary

Update server-side code to be more es6-ey

Review Request #9541 — Created Jan. 26, 2018 and submitted Feb. 1, 2018, 12:57 p.m.

Information

Owner

david

Repository

student-sonar

Branch

master

Bugs

Depends On

Commit

4d18c72...

Reviewers

Groups

student-sonar

People

Description

This change changes the server-side code in Sonar to use ES6 modules and
more idiomatic and consistent syntax. The only thing left that's still
using module.exports is lib/schema, which manipulates the exports at
runtime (something ES6 modules can't do). A future refactor will address
this.

As part of this, I've updated the aws-sdk and memjs dependencies to
get newer versions which support promises. This allows us to move
entirely over to native promises, eliminating the Q library.

Testing Done

Went through the various functionality of the server and verified that
things work as expected.

Issues

Description	From	Last Updated
Col: 19 Missing 'new' prefix when invoking a constructor.	reviewbot	Jan. 26, 2018, 5:20 p.m.
Col: 25 Missing 'new' prefix when invoking a constructor.	reviewbot	Jan. 26, 2018, 5:20 p.m.
Col: 19 Missing 'new' prefix when invoking a constructor.	reviewbot	Jan. 26, 2018, 5:20 p.m.
Col: 19 Missing 'new' prefix when invoking a constructor.	reviewbot	Jan. 26, 2018, 5:20 p.m.
Col: 8 'https' is defined but never used.	reviewbot	Jan. 26, 2018, 5:20 p.m.
export default ? Your description says module.exports is only required for lib/schema. (Same below).	brennie	Jan. 31, 2018, 9 a.m.
Is this a constructor (I see this.memoize). If so, why not use a class?	brennie	Jan. 31, 2018, 9:01 a.m.
export default ?	brennie	Jan. 31, 2018, 9 a.m.
export function init(mongoose) over module.exports ?	brennie	Jan. 30, 2018, 9:17 p.m.
Is this a constructor? Why not use a class here?	brennie	Jan. 31, 2018, 9:01 a.m.

flake8 passed.

JSHint failed.

JSHint

lib/api.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 19
 Missing 'new' prefix when invoking a constructor.
```
lib/api.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 25
 Missing 'new' prefix when invoking a constructor.
```
lib/api.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 19
 Missing 'new' prefix when invoking a constructor.
```
lib/api.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 19
 Missing 'new' prefix when invoking a constructor.
```
lib/slack.js (Diff revision 1)
The issue has been resolved. Show all issues
```
Col: 8
 'https' is defined but never used.
```

Commit:

-	a48dd5af42e7e0d3139316159d298bce496228dd
+	83151402e920a1d7d3e4b425c6b5be317e0fa0b9

Diff:

Revision 2 (+281 -378)

Show changes

	.gitignore
	package.json
	package-lock.json
	server.js
	lib/api.js
	lib/asana.js
	lib/cache.js
	lib/config.js
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

lib/api.js (Diff revision 2)

The issue has been resolved. Show all issues

export default ? Your description says module.exports is only required for lib/schema. (Same below).

lib/cache.js (Diff revision 2)

The issue has been dropped. Show all issues

Is this a constructor (I see this.memoize). If so, why not use a class?

david Jan. 31, 2018, 9:01 a.m.

It is a constructor. I theoretically could use a class but I want to think more about the server architecture so for now I don't want to touch it too much.

lib/routes.js (Diff revision 2)
The issue has been resolved. Show all issues
```
export default ?
```

lib/schema.js (Diff revision 2)

The issue has been dropped. Show all issues

export function init(mongoose) over module.exports ?

david Jan. 31, 2018, 9:01 a.m.
```
This is the one that definitely can't.
```

lib/slack.js (Diff revision 2)
The issue has been dropped. Show all issues
```
Is this a constructor? Why not use a class here?
```

Commit:

-	83151402e920a1d7d3e4b425c6b5be317e0fa0b9
+	4d18c729b84daf649b3cd031b7066f8b6a833e27

Diff:

Revision 3 (+286 -383)

Show changes

	.gitignore
	package.json
	package-lock.json
	server.js
	lib/api.js
	lib/asana.js
	lib/cache.js
	lib/config.js
	5 more

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

Status: Closed (submitted)

Change Summary:

Pushed to master (22c2b70)