Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Make verbosity level mandatory with telemetry opt #5057

Merged
merged 8 commits into from
Apr 5, 2020

Conversation

pscott
Copy link
Contributor

@pscott pscott commented Feb 25, 2020

As mentioned in #4372 , this PR makes the verbosity level mandatory in the --telemetry-urls option, instead of defaulting to 0.

There are a couple of points I would like to clarify:

  1. I'm not sure this is considering breaking the API? Or should I flag it as B2-breaksapi?
  2. I don't think this PR changes anything to Polkadot but I'm not sure?
  3. I haven't managed to find the place where we check that the VERBOSITY_LEVEL is not bigger than the maximum verbosity_level. Maybe there's no point in doing so, but it might be better practice to let the end user know that when he specifies a verbosity-level bigger than 9, it doesn't actually exist?
  4. As for verbosity level, I've found that the constant SUBSTRATE_DEBUG is declared here
    pub const SUBSTRATE_DEBUG: &str = "9";
    but I have not managed to find where it is USED in the code. Is it just here for export purposes?

@pscott pscott added the A0-please_review Pull request needs code review. label Feb 25, 2020
@pscott pscott self-assigned this Feb 25, 2020
@parity-cla-bot
Copy link

It looks like @pscott hasn't signed our Contributor License Agreement, yet.

The purpose of a CLA is to ensure that the guardian of a project's outputs has the necessary ownership or grants of rights over all contributions to allow them to distribute under the chosen licence.
Wikipedia

You can read and sign our full Contributor License Agreement at the following URL: https://cla.parity.io

Once you've signed, please reply to this thread with [clabot:check] to prove it.

Many thanks,

Parity Technologies CLA Bot

@pscott
Copy link
Contributor Author

pscott commented Feb 25, 2020

[clabot:check]

@parity-cla-bot
Copy link

It looks like @pscott signed our Contributor License Agreement. 👍

Many thanks,

Parity Technologies CLA Bot

@pscott pscott marked this pull request as ready for review February 26, 2020 09:52
@pscott pscott requested review from tomaka and tomusdrw February 26, 2020 10:07
tomaka
tomaka previously requested changes Feb 26, 2020
@pscott pscott added A3-in_progress Pull request is in progress. No review needed at this stage. and removed A0-please_review Pull request needs code review. labels Feb 26, 2020
@pscott
Copy link
Contributor Author

pscott commented Feb 26, 2020

Currently waiting on #5069 to be reviewed (and maybe) merged, and then will be able to address comments and get ready for review. :D

@pscott pscott added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Mar 26, 2020
@pscott pscott requested review from tomaka and bkchr March 26, 2020 19:35
@pscott pscott dismissed tomaka’s stale review March 27, 2020 08:37

Review has been addressed but is not showing up as addressed

@gavofyork gavofyork merged commit 8cee4fe into paritytech:master Apr 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CLI: Telemetry URL & Verbosity Flags
6 participants