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.

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

refactor: simplify connection encrypt option #1001

Merged
merged 1 commit into from Nov 5, 2019

Conversation

@davidjb
Copy link
Contributor

davidjb commented Nov 5, 2019

This moves the default for config.options.encrypt up into the base
configuration object, simplifying the setup and making it clearer
that the default is encrypt: true (and a minute bit faster).

Behaviour remains identical after this restructure.


Before submitting a PR :

  1. Ensure your fork is created from master branch of the repository.
  2. Run npm install in the root folder.
  3. After bug fix/code change, ensure all the existing tests and new tests (if any) pass (npm run-script test-all). During development, to run individual test use node_modules/nodeunit test/<test_file.js> -t <test_name>.
  4. Build the driver (npm run build).
  5. Run eslint and flow typechecker (npm run lint).
  6. Run commitlint (node_modules/.bin/commitlint --from origin/master --to HEAD). Refer commit conventions and commit rules.

Thank you for Contributing!

This moves the default for `config.options.encrypt` up into the base
configuration object, simplifying the setup and making it clearer
that the default is `encrypt: true`.

Behaviour remains identical after this restructure.
@codecov

This comment has been minimized.

Copy link

codecov bot commented Nov 5, 2019

Codecov Report

Merging #1001 into master will decrease coverage by 0.95%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1001      +/-   ##
==========================================
- Coverage   78.61%   77.65%   -0.96%     
==========================================
  Files          86       86              
  Lines        4231     4230       -1     
  Branches      778      778              
==========================================
- Hits         3326     3285      -41     
- Misses        631      673      +42     
+ Partials      274      272       -2
Impacted Files Coverage Δ
src/connection.ts 59.56% <ø> (-3.66%) ⬇️
src/incoming-message-stream.ts 92.3% <0%> (-5.13%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1fbed59...2a9c7b1. Read the comment docs.

@arthurschreiber arthurschreiber merged commit ab475d8 into tediousjs:master Nov 5, 2019
4 of 5 checks passed
4 of 5 checks passed
codecov/project 77.65% (-0.96%) compared to 1fbed59
Details
LGTM analysis: JavaScript No new or fixed alerts
Details
codecov/patch Coverage not affected when comparing 1fbed59...2a9c7b1
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.