Skip to content

Don't use --quiet verbosity by default when running dylint#711

Merged
HCastano merged 5 commits intomasterfrom
hc-fix-output-json
Aug 30, 2022
Merged

Don't use --quiet verbosity by default when running dylint#711
HCastano merged 5 commits intomasterfrom
hc-fix-output-json

Conversation

@HCastano
Copy link
Copy Markdown
Contributor

@HCastano HCastano commented Aug 26, 2022

It looks like as part of #703 we added a --quiet flag
when running the linter. The --output-json flag also adds a --quiet flag resulting in
a failed run due to duplicate flags.

❮ RUSTC_BOOTSTRAP=1 cargo +stable contract build --output-json
error: The argument '--quiet' was provided more than once, but cannot be used multiple times

USAGE:
    cargo dylint [OPTIONS] [NAMES]... [-- <ARGS>...]

For more information try --help
ERROR: `"/.../bin/cargo" "dylint" "--lib=ink_linting" "--quiet" "--quiet"` failed with exit code: Some(2)

I figured that we probably don't want to run the linting step when running with
--output-json anyways, and by skipping that step we can avoidget this duplicate flag
conflict in the first place.

I've instead changed this PR to not run dylint with --quiet, and instead it'll use the verbosity
specified by the user. In your PR, @athei, you specified --quiet runs by default. Is there any
reason for this?

@athei
Copy link
Copy Markdown
Contributor

athei commented Aug 26, 2022

Why would we not want linting when using json output? Couldn't something like ink_playground use it to show linting errors to users?

@cmichi
Copy link
Copy Markdown
Collaborator

cmichi commented Aug 26, 2022

I figured that we probably don't want to run the linting step when running with --output-json anyways

Could you explain your thinking here? How would one enable linting for --output-json then?

A use-case is to have CI build a contract with --output-json with the intention of extracting the sizes and artifact paths. In that case you would require a way to run the linting as part of the build to ensure that the contract is fine in all ways.

@HCastano
Copy link
Copy Markdown
Contributor Author

My thinking was along the lines of our current use case in the waterfall, which doesn't really need any linting info, but I agree we should still lint contracts

@HCastano HCastano changed the title Skip linting when running with --output-json Don't use --quiet verbosity by default when running dylint Aug 29, 2022
@athei
Copy link
Copy Markdown
Contributor

athei commented Aug 30, 2022

In your PR, @athei, you specified --quiet runs by default. Is there any reason for this?

Yes. Since we build the lint now as part of the contract build (and not when building cargo contract) we get some wonky warnings during this build. I don't really want to show those warnings. They are annoying and confusing.

Maybe you can set the default verbosity for linting to quiet?

@@ -341,7 +341,7 @@ fn exec_cargo_dylint(crate_metadata: &CrateMetadata, verbosity: Verbosity) -> Re
};
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could do something like

let verbosity = match verbosity {
  Verbosity::Verbose => Verbosity::Default,
  Verbosity::Default | Verbosity::Quiet => Verbosity::Quiet
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I like this 👍

@HCastano HCastano merged commit 3d24c62 into master Aug 30, 2022
@HCastano HCastano deleted the hc-fix-output-json branch August 30, 2022 18:59
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