Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(refactor) lib/db.js into es6 classes #566

Merged
merged 3 commits into from
Jul 18, 2016
Merged

(refactor) lib/db.js into es6 classes #566

merged 3 commits into from
Jul 18, 2016

Conversation

jniles
Copy link
Collaborator

@jniles jniles commented Jul 7, 2016

This commit rewrites the db modules as es6 classes. The module is API-compatible with the previous version of db. The transaction logic has been broken out into a separate class Transaction for testability. The files db/index.js and db/transaction.js now replace db.js.

There is no longer any db.initialise() method. The first `require() will initialise a database connection from environmental variables.

The documentation has improved drastically. Each method is now well documented using JSDoc standards.

Finally, nicer errors have been implemented for API/integration tests. This makes the testing library log the HTTP method and URL in case programmers forget to accurately describe their assertions.

Thank you for contributing!

Before submitting this pull request, please verify that you have:

  • Run your code through JSHint. Check out our styleguide.
  • Run integration tests.
  • Run end-to-end tests.
  • Accurately described the changes your are making in this pull request.

For a more detailed checklist, see the official review checklist that this PR will be evaluated against.

Ensuring that the above checkboxes are completed will help speed the review process and help build a stronger application. Thanks!

@jniles jniles added the Refactor label Jul 7, 2016
@jniles jniles force-pushed the refactor-lib-database branch 2 times, most recently from c16b03a to c4c89e8 Compare July 7, 2016 13:53
sfount
sfount reviewed Jul 9, 2016
View reviewed changes
gulpfile.js
@@ -163,7 +161,7 @@ gulp.task('client-compile-vendor', function () {
});


// minify the client css styles via minifycss
// minify the client css styles via cssnano
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😆

@jniles jniles force-pushed the refactor-lib-database branch 3 times, most recently from 0dc9d9c to aafecbe Compare July 12, 2016 15:40
@jniles
Copy link
Collaborator Author

jniles commented Jul 12, 2016

@sfount, can I get a review?

Jonathan Niles added 3 commits July 14, 2016 11:43
refactor(db): es6 rewrite of db mods
7498d68 
This commit rewrites the db modules as es6 classes.  The module is
API-compatible with the previous version of `db`.  The transaction logic
has been broken out into a separate class `Transaction` for testability.
The files `db/index.js` and `db/transaction.js` now replace `db.js`.

There is no longer any `db.initialise()` method.  The first `require()`
will initialise a database connection from environmental variables.

The documentation has improved drastically.  Each method is now well
documented using JSDoc standards.
deps(gulp-flatten): rm unused dependency
bc607ad 
This commit removes the unused dependency on gulp-flatten.
test(api): ensure nice logs for api test failures
9e1fca9 
This commit makes the api tests log their failures in a more explicit
manner, rather than relying on the programmer to write the proper text
in to the `it()` statement.  Hopefully this will save time in future bug
hunts.
@jniles
Copy link
Collaborator Author

jniles commented Jul 17, 2016

@sfount, can I get a review?

@sfount
Copy link
Contributor

sfount commented Jul 18, 2016

This is a huge improvement to the database connector. It is worrying that there are no unit tests to verify that these changes provide the same functionality however the integration tests heavily relying on db go a long way.

In the future we could either look into an additional library or write some tests to cover this functionality.

@sfount sfount merged commit 28db945 into IMA-WorldHealth:master Jul 18, 2016
@jniles jniles deleted the refactor-lib-database branch July 18, 2016 08:02
bors bot added a commit that referenced this pull request Oct 1, 2018
@bors @greenkeeper
Merge #3212
2ce8ce3 
3212: Update commitizen to the latest version 🚀 r=jniles a=greenkeeper[bot]


## The devDependency [commitizen](https://github.com/commitizen/cz-cli) was updated from `2.10.1` to `3.0.0`.
This version is **not covered** by your **current version range**.

If you don’t accept this pull request, your project will work just like it did before. However, you might be missing out on a bunch of new features, fixes and/or performance improvements from the dependency update.

---

<details>
<summary>Release Notes for v3.0.0</summary>

<p>&lt;a name"3.0.0"&gt;</p>
<h2>3.0.0 (2018-10-01)</h2>
<h4>Bug Fixes</h4>
<ul>
<li>resolve linux build issues, add git exit code 128 warning (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/c3764c8c">c3764c8c</a>)</li>
<li><strong>deps:</strong>
<ul>
<li>update dependency glob to v7.1.2 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326327664" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#506" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/506">#506</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/aa824960">aa824960</a>)</li>
<li>update dependency lodash to v4.17.10 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326338773" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#507" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/507">#507</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/4bda9435">4bda9435</a>)</li>
<li>update dependency find-root to v1.1.0 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326327598" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#505" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/505">#505</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/67a4e16a">67a4e16a</a>)</li>
<li>update dependency dedent to v0.7.0 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326314146" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#504" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/504">#504</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a21b674f">a21b674f</a>)</li>
<li>update dependency cz-conventional-changelog to v2.1.0 (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="326314071" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#503" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/503">#503</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/43d54de1">43d54de1</a>)</li>
</ul>
</li>
<li><strong>package:</strong> Remove opencollective post-install hook (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="356422024" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#561" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/561">#561</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c234d">a70c234d</a>)</li>
</ul>
<h4>Features</h4>
<ul>
<li>Drop support for Node.js &lt;6.x, update dependencies (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="361350875" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#566" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/566">#566</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c063b">a70c063b</a>)</li>
<li>support initialization with yarn. (<a class="issue-link js-issue-link" data-error-text="Failed to load issue title" data-id="349367370" data-permission-text="Issue title is private" data-url="commitizen/cz-cli#549" href="https://urls.greenkeeper.io/commitizen/cz-cli/pull/549">#549</a>) (<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d5652154">d5652154</a>, closes <a href="https://urls.greenkeeper.io/commitizen/cz-cli/issues/527">#527</a>, <a href="https://urls.greenkeeper.io/commitizen/cz-cli/issues/527">#527</a>)</li>
</ul>
<h4>Breaking Changes</h4>
<ul>
<li>Older versions of Node.js are no longer supported</li>
</ul>
<p>Includes most updates with the exception of semantic-release which breaks on windows. To be investigated in a later release.<br>
(<a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c063b">a70c063b</a>)</p>
</details>

<details>
<summary>Commits</summary>
<p>The new version differs by 23 commits.</p>
<ul>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c063b06dbdf41af322dab06f83bfddd69149b"><code>a70c063</code></a> <code>feat: Drop support for Node.js &lt;6.x, update dependencies (#566)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/0d76887b7853f673949cf7aee04653b90b4dda7b"><code>0d76887</code></a> <code>chore: remove sign from windows build</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/1546df4856dc8c4aa781e9e0ea755613647642ca"><code>1546df4</code></a> <code>chore: assume yarn in tests</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/c3764c8c1d2b05597a6c604c13b6a11731c47281"><code>c3764c8</code></a> <code>fix: resolve linux build issues, add git exit code 128 warning</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/f1150c83a62048831fe6d94836f52242fdc9831e"><code>f1150c8</code></a> <code>chore: Create jobs build yml</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/f0595edca921b68913e2ce2ca1bbad75abe71515"><code>f0595ed</code></a> <code>chore: add azure jobs</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/4693079d0400ad1a914af86c6027f83d18cde74d"><code>4693079</code></a> <code>chore: Set up CI with Azure Pipelines</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d5652154fa1bb41c1b1c84f20c620ee64ca32c78"><code>d565215</code></a> <code>feat: support initialization with yarn. fixes #527 (#549)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/a70c234d8471af147cc9f7d9d090b6ba2192eb17"><code>a70c234</code></a> <code>fix(package): Remove opencollective post-install hook (#561)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d5b8bc587770e4a7efd6a36962abaa0425931181"><code>d5b8bc5</code></a> <code>docs(readme): add npx use examples (#532)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/d103b10f56121a8617ac9ac882338338417b0947"><code>d103b10</code></a> <code>chore(ci): test on Node.js 10.x (#531)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/41a921b002d1627cfedd4bb5745a27d8a47a5dfe"><code>41a921b</code></a> <code>BREAKING CHANGE: Remove Node 0.12 support (#524)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/aa8249605d45c1e95f57be7c832321212b0e8d1c"><code>aa82496</code></a> <code>fix(deps): update dependency glob to v7.1.2 (#506)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/4bda94350625e54ca862cd143432de1912caea27"><code>4bda943</code></a> <code>fix(deps): update dependency lodash to v4.17.10 (#507)</code></li>
<li><a href="https://urls.greenkeeper.io/commitizen/cz-cli/commit/67a4e16a88c7925f3199996002ba8da37e420cff"><code>67a4e16</code></a> <code>fix(deps): update dependency find-root to v1.1.0 (#505)</code></li>
</ul>
<p>There are 23 commits in total.</p>
<p>See the <a href="https://urls.greenkeeper.io/commitizen/cz-cli/compare/5838ce007c1af2e71ef6ae68efce7682d59e9061...a70c063b06dbdf41af322dab06f83bfddd69149b">full diff</a></p>
</details>

<details>
  <summary>FAQ and help</summary>

  There is a collection of [frequently asked questions](https://greenkeeper.io/faq.html). If those don’t help, you can always [ask the humans behind Greenkeeper](https://github.com/greenkeeperio/greenkeeper/issues/new).
</details>

---


Your [Greenkeeper](https://greenkeeper.io) bot 🌴



Co-authored-by: greenkeeper[bot] <greenkeeper[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Refactor
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jniles @sfount