Merge new features from the riff develop branch#62
Open
mlippert wants to merge 39 commits intoHumanDynamics:developmentfrom
Open
Merge new features from the riff develop branch#62mlippert wants to merge 39 commits intoHumanDynamics:developmentfrom
mlippert wants to merge 39 commits intoHumanDynamics:developmentfrom
Conversation
ran tests w/ node version 8 and they all pass
* changed node engine to allow later versions * updated version specifier of dependencies to use ^ so later patches and minor versions will be used. * removed unreferenced package dependencies * removed duplicate packages in devDependencies * mongodb package is only referenced by tests, so moved it to devDependencies
…etc) remove ENV settings from rhythm-server Dockerfile because the .env file is loaded via the dotenv package required by the relevant package json scripts. It is also loaded by heroku when this application is deployed there.
…prompt - rather than using root in those containers, use the "node" user - set colored interactive prompt that should help recognizing when running a shell in the container add a Makefile for ease of executing some commands, not to replace the npm scripts, (they should be kept in sync when duplicated) but to supplement it, such as for starting up the interactive docker container development environment.
…s.remove method the feathers-hooks package has been integrated into feathers 3. It seems the remove method it used to provide was in feathers-hooks-common, but deprecated and removed from there, replaced by discard.
https://github.com/feathersjs/authentication/blob/master/docs/migrating.md `verifyToken` was removed, replaced by `auth.hooks.authenticate('jwt')` although the docs seem to say doing that everywhere isn't necessary anymore. But it's not yet clear to me how and why that would be true.
- added channels.js as described in the migration guide - use new authentication paradigm - fix attaching service hooks
… authentication channel
- npm test is actually running, fails on authentication now, so that's next
- mongodb is only used in global-before.js to "cleanup by dropping the database" - turns out the previous remove hook used to remove the password field, did not remove it if the call came from the server. Added this functionality back. - updated other feathers client uses as they should be done when running from node.
I'd like to get this into a config file, but for now the logging was getting in the way of seeing the tests running.
…ev package - d3 has been broken into many modules, so I've only installed the ones currently being used. - I'm not positive that nodemailer still works, it will require testing w/ an smtp server available - I updated the promise package, but I think we should remove it altogether and use native Promises at this point - updating mocha has exposed an issue with the tests. mocha is hanging when the tests "finish" because they aren't cleaning up properly after themselves. See https://boneskull.com/mocha-v4-nears-release/#mochawontforceexit more debugging of the tests is needed to figure out how to fix them.
exiting from the rhythm-server container shouldn't abort the following commands in the recipe. Added a new command to stop running containers (namely mongo)
Copied the templates used by [KeePassXC](https://github.com/keepassxreboot/keepassxc) which seem like good templates to start with.
…ining packages - Update to latest major version of eslint - modify package.json scripts removing standard and adding eslint
Mostly attempt to choose style rules to match existing style For currently inconsistent styling, choose what I like best that still fits with the existing style. Set errors/warnings level based on whether I think it reveals a potential problem. Further review may warrant changing these settings or designations.
uncovered more areas that need fixing.
moved mocha env to the test config modified the no-shadow to allow shadowing `done`
including vim support for running `:make vim-lint` to use the vim quickfix window.
If we get rid of all of the errors and warnings, this file can be removed from the repository. Meantime this should help prevent new errors/warnings while working on the existing set.
- remaining errors are no-shadow and consistent-return and changes will be bigger to fix them. - added eslint exceptions to set app = this (consistent-this) because feathers calls those functions w/ the feathers app as the context. source file changes to match lint rules
What remains needs more review examining the code more closely to see how it should be written. The errors are potentially a source of bugs, the warnings are merely informative for good code style but individual cases may have good reasons and need an eslint override in the file.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Riff develop branch has 2 new features:
See those pull requests for details of the changes.