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

Enable floating promises check everywhere (fix issues) #1115

Merged
merged 4 commits into from
Sep 28, 2019

Conversation

mcortesi
Copy link
Contributor

Description

Removes floagint promises on all packages that inherit from tslint
parent config.

Only in mobile package, the config is overrided as a "warning".

Also: updates tslint & tslint rule set to their last versions.

Tested

yarn lint

Other changes

Upgrade tslint

Related issues

@@ -22,8 +21,8 @@ export class AddressRegistry {
async addressFor(contract: CeloContract): Promise<Address> {
if (!this.cache.has(contract)) {
debug('Fetching address from Registry for %s', contract)
const hash = Web3.utils.soliditySha3({ type: 'string', value: contract })
const address = await this.registry.methods.getAddressFor(hash).call()
// const hash = Web3.utils.soliditySha3({ type: 'string', value: contract })
Copy link
Contributor

Choose a reason for hiding this comment

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

why comment this out?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for catching this.. my commit included some other work that should not be here.

Now that's fixed

packages/verifier/src/components/HomeScreen/HomeScreen.tsx Outdated Show resolved Hide resolved
Removes floagint promises on all packages that inherit from tslint
parent config.

Only in mobile package, the config is overrided as a "warning".

Also: updates tslint & tslint rule set to their last versions.
@codecov
Copy link

codecov bot commented Sep 26, 2019

Codecov Report

Merging #1115 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1115   +/-   ##
=======================================
  Coverage   66.59%   66.59%           
=======================================
  Files         257      257           
  Lines        7394     7394           
  Branches      494      430   -64     
=======================================
  Hits         4924     4924           
- Misses       2373     2375    +2     
+ Partials       97       95    -2
Flag Coverage Δ
#mobile 66.59% <ø> (ø) ⬆️
Impacted Files Coverage Δ
packages/mobile/src/utils/formatting.ts 87.5% <0%> (ø) ⬆️
packages/mobile/src/identity/reducer.ts 41.66% <0%> (ø) ⬆️

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 0c948a7...44d431d. Read the comment docs.

Copy link
Contributor

@jmrossy jmrossy left a comment

Choose a reason for hiding this comment

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

Very nice!

@jmrossy jmrossy assigned mcortesi and unassigned jmrossy and cmcewen Sep 27, 2019
@mcortesi mcortesi added the automerge Have PR merge automatically when checks pass label Sep 27, 2019
@ashishb ashishb merged commit 9a3b769 into master Sep 28, 2019
@ashishb ashishb deleted the mc/floating-promises branch September 28, 2019 01:01
aaronmgdr added a commit that referenced this pull request Oct 1, 2019
* master: (31 commits)
  Upgrade to Node 10 (#1148)
  [faucet] Add custom metrics (#1143)
  Add IdentityMetadata to Contractkit (#1126)
  [Wallet] Local currency v1.1 (#1137)
  Add attestation-service deploy (#1128)
  [Wallet] A few docs and build cleanup (#1138)
  [CircleCI]Add comment on how to fix lint checks (#1134)
  2019-09-30 integration deployment (#1149)
  Update web3 provider to new integration url (#1151)
  [celotool]Add fast mode to celotool invite (#1135)
  Revert "Feature/909 proxy delegatecall" (#1146)
  Use contractkit in notification service (#1118)
  Feature/909 proxy delegatecall (#1003)
  integration deployment for 2019-09-29 (#1139)
  Add instructions for npm publication to tag commit (#1117)
  Client Logs Data Flow script update (#1055)
  Deploying latest proxy code in genesis (#1122)
  Enable floating promises check everywhere (fix issues) (#1115)
  [cli] Solution for build error contractkit on Linux 19.04 distro (#960)
  [Wallet] Merge back changes made for mx pilot (#1113)
  ...

# Conflicts:
#	yarn.lock
aaronmgdr added a commit that referenced this pull request Oct 1, 2019
* master: (35 commits)
  [Wallet] Network fee in transaction feed (#1145)
  New About Page Cover (#905)
  Upgrade to Node 10 (#1148)
  [faucet] Add custom metrics (#1143)
  Add IdentityMetadata to Contractkit (#1126)
  [Wallet] Local currency v1.1 (#1137)
  Add attestation-service deploy (#1128)
  [Wallet] A few docs and build cleanup (#1138)
  [CircleCI]Add comment on how to fix lint checks (#1134)
  2019-09-30 integration deployment (#1149)
  Update web3 provider to new integration url (#1151)
  [celotool]Add fast mode to celotool invite (#1135)
  Revert "Feature/909 proxy delegatecall" (#1146)
  Use contractkit in notification service (#1118)
  Feature/909 proxy delegatecall (#1003)
  integration deployment for 2019-09-29 (#1139)
  Add instructions for npm publication to tag commit (#1117)
  Client Logs Data Flow script update (#1055)
  Deploying latest proxy code in genesis (#1122)
  Enable floating promises check everywhere (fix issues) (#1115)
  ...

# Conflicts:
#	packages/web/src/about/About.tsx
#	packages/web/src/about/images/index.ts
#	packages/web/static/locales/en/about.json
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Have PR merge automatically when checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

possible bug in faucet
4 participants