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

Update PostgresStorageAdapter.js #6275

Merged
merged 4 commits into from
Dec 16, 2019
Merged

Update PostgresStorageAdapter.js #6275

merged 4 commits into from
Dec 16, 2019

Conversation

vitaly-t
Copy link
Contributor

@vitaly-t vitaly-t commented Dec 11, 2019

Improving use of the await/async notation in relation to pg-promise, and in general.

Improving use of the `await.async` notation in relation to `pg-promise`, and in general.
Correcting some results.
@codecov
Copy link

codecov bot commented Dec 11, 2019

Codecov Report

Merging #6275 into master will increase coverage by <.01%.
The diff coverage is 80%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6275      +/-   ##
==========================================
+ Coverage   93.81%   93.82%   +<.01%     
==========================================
  Files         169      169              
  Lines       11564    11564              
==========================================
+ Hits        10849    10850       +1     
+ Misses        715      714       -1
Impacted Files Coverage Δ
...dapters/Storage/Postgres/PostgresStorageAdapter.js 97.01% <80%> (ø) ⬆️
src/RestWrite.js 93.42% <0%> (-0.33%) ⬇️
src/Adapters/Storage/Mongo/MongoStorageAdapter.js 93.44% <0%> (+0.7%) ⬆️

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 4f1d3b0...7d6adfd. Read the comment docs.

@dplewis
Copy link
Member

dplewis commented Dec 12, 2019

@vitaly-t Thanks for the PR, it looks good to me. Is there a reason why this is an improvement? I always use async / await like this.

@vitaly-t
Copy link
Contributor Author

@dplewis The initial idea was to improve methods such as createClass and createTable, to stop returning things that are not needed, but I could not finish in the end, because the dependent tests are incorrect, and would need to be updated also.

Methods such as those, resolve with null or undefined, representing nothing. The tests should verify that the method resolves, and not test what it returns, or expect undefined. Then the methods can be corrected. There is no point testing the first methods returning [null, null, null] from a batch, or just null in other cases (from result of method .one).

@dplewis
Copy link
Member

dplewis commented Dec 12, 2019

Oh I see, you are right they shouldn't return.

The adapter should conform to the StorageAdapter

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

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

LGTM!

@dplewis dplewis merged commit 2d665c9 into parse-community:master Dec 16, 2019
UnderratedDev pushed a commit to UnderratedDev/parse-server that referenced this pull request Mar 21, 2020
* Update PostgresStorageAdapter.js

Improving use of the `await.async` notation in relation to `pg-promise`, and in general.

* Update PostgresStorageAdapter.js

* Update PostgresStorageAdapter.js

Correcting some results.

* Update PostgresStorageAdapter.js
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants