Skip to content

feat: allow setting tx mining service apikey #175

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

Merged
merged 9 commits into from
Apr 19, 2022

Conversation

luislhl
Copy link
Contributor

@luislhl luislhl commented Apr 6, 2022

Acceptance Criteria

  • It should be possible to set an api-key in the config to be used for tx-mining-service

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@luislhl luislhl self-assigned this Apr 6, 2022
@luislhl luislhl requested a review from pedroferreira1 as a code owner April 6, 2022 15:21
@codecov
Copy link

codecov bot commented Apr 6, 2022

Codecov Report

Merging #175 (5fdfe8f) into dev (7a29078) will increase coverage by 0.06%.
The diff coverage is 57.14%.

@@            Coverage Diff             @@
##              dev     #175      +/-   ##
==========================================
+ Coverage   85.78%   85.84%   +0.06%     
==========================================
  Files          19       19              
  Lines         647      650       +3     
  Branches      132      133       +1     
==========================================
+ Hits          555      558       +3     
  Misses         83       83              
  Partials        9        9              
Impacted Files Coverage Δ
src/controllers/index.controller.js 72.50% <0.00%> (ø)
src/index.js 71.42% <100.00%> (+4.76%) ⬆️

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 7a29078...5fdfe8f. Read the comment docs.

@luislhl luislhl requested a review from andreabadesso April 7, 2022 14:46
@pedroferreira1
Copy link
Member

The code for me is good, just two considerations:

  1. I think we should wait for @tuliomir's refactor PR (Refactor of ExpressJS files #174). It changed some stuff in the src.index.js file and may change a bit this PR as well.
  2. We need to wait for the lib bump before merging

@luislhl luislhl force-pushed the feat/allow-setting-tx-mining-service-apikey branch from 08df595 to b3e8a5c Compare April 8, 2022 19:56
… into feat/allow-setting-tx-mining-service-apikey
@luislhl
Copy link
Contributor Author

luislhl commented Apr 18, 2022

I'm ignoring Codecov report this time because they are complaining about some console.log that I have replaced with console.error. This can be seen in their report link.

@luislhl luislhl merged commit 326bc7b into dev Apr 19, 2022
@luislhl luislhl deleted the feat/allow-setting-tx-mining-service-apikey branch April 19, 2022 14:14
This was referenced Apr 19, 2022
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.

3 participants