Skip to content

feat: add metrics service - #2199

Merged
bors[bot] merged 1 commit into
nervosnetwork:developfrom
chaoticlonghair:pr/add-metrics-service
Aug 12, 2020
Merged

bors[bot] merged 1 commit into
nervosnetwork:developfrom
chaoticlonghair:pr/add-metrics-service

Conversation

@chaoticlonghair

@chaoticlonghair chaoticlonghair commented Jul 30, 2020

Copy link
Copy Markdown
Contributor

quake
quake previously approved these changes Aug 3, 2020
Comment thread util/metrics-service/src/config.rs Outdated
Comment thread util/metrics-service/src/config.rs Outdated
Comment thread util/metrics-service/src/config.rs Outdated
@chaoticlonghair

Copy link
Copy Markdown
Contributor Author

The last "force-pushed" just removed a useless comment.

keroro520
keroro520 previously approved these changes Aug 4, 2020
Comment thread util/app-config/src/app_config.rs Outdated
Comment thread util/metrics-service/src/config.rs Outdated
Comment thread util/metrics-service/src/config.rs Outdated
Comment thread util/metrics-service/src/service.rs Outdated
@doitian

doitian commented Aug 5, 2020

Copy link
Copy Markdown
Contributor

Since it heavily depends on a 3rd-party crate, please and link and some introductions in the PR description.

Comment thread util/metrics/src/lib.rs Outdated
@chaoticlonghair chaoticlonghair added the s:hold Status: Put this issue on hold. label Aug 5, 2020
@chaoticlonghair

Copy link
Copy Markdown
Contributor Author

Hold for updating, according to review suggestions.

@doitian

doitian commented Aug 11, 2020

Copy link
Copy Markdown
Contributor

@yangby-cryptape Ready to unhold?

doitian
doitian previously approved these changes Aug 11, 2020
@chaoticlonghair

chaoticlonghair commented Aug 11, 2020

Copy link
Copy Markdown
Contributor Author

@yangby-cryptape Ready to unhold?

Everything is ready! And I have been using the commits in my laptop.
I will rebase and force push those commits in this PR after #2220 merged.

bors Bot added a commit that referenced this pull request Aug 11, 2020
2220: refactor: split logger config and service r=TheWaWaR,doitian,keroro520 a=yangby-cryptape

_Originally posted by @doitian in #2199 (comment)

- Split logger config and service.
- Let `ckb-app-config` only be dependent on `ckb-logger-config`.
- Move the function which checks identifier into `ckb-util`.

Co-authored-by: Boyu Yang <yangby@cryptape.com>
@chaoticlonghair
chaoticlonghair dismissed stale reviews from doitian and keroro520 via 0db57da August 12, 2020 01:55
@chaoticlonghair chaoticlonghair removed the s:hold Status: Put this issue on hold. label Aug 12, 2020
@chaoticlonghair

chaoticlonghair commented Aug 12, 2020

Copy link
Copy Markdown
Contributor Author

Updated according to review suggestions.

The most major change is: create a config crate so that app config does not depend on services.

@chaoticlonghair

Copy link
Copy Markdown
Contributor Author

@quake I updated the format of dependencies' versions in Cargo.toml in the last force-push.

@chaoticlonghair

Copy link
Copy Markdown
Contributor Author

bors r=quake,keroro520

@bors

bors Bot commented Aug 12, 2020

Copy link
Copy Markdown
Contributor

Build succeeded:

  • continuous-integration/travis-ci/push

@bors
bors Bot merged commit 1e91c35 into nervosnetwork:develop Aug 12, 2020
@chaoticlonghair
chaoticlonghair deleted the pr/add-metrics-service branch September 11, 2020 06:14
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.

4 participants