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

igluctl: switch severity level to skip checks #232

Closed
chuwy opened this issue Jan 27, 2017 · 4 comments
Closed

igluctl: switch severity level to skip checks #232

chuwy opened this issue Jan 27, 2017 · 4 comments
Assignees

Comments

@chuwy
Copy link
Contributor

chuwy commented Jan 27, 2017

Currently severity levels assume that we can have better or worse (high quality/low quality) checks, whereas it really not high/low, but include/exclude. User should have an ability to disable/enable each particular check if he is sure that absence of maxLength is fine.

My idea is to switch severity levels with checks option (or even --skip-checks), so user will have explicitly skip some lint if he wants: igluctl lint /path/ --skip-checks optionalNull,implicitMax, where optionalNull disables #231 check and implicitMax disables #175.

@chuwy
Copy link
Contributor Author

chuwy commented Jan 19, 2018

Hey @oguzhanunlu, here's another piece of info.

Yes, we're going to get rid of notion of levels entirely as well as of --severityLevel option and instead skip each linter individually. Otherwise your approach looks quite correct.

You can have a map of linters:

val allLinters: Map[String, Linter] = Map(
  "minimumMaximum" -> lintMinimumMaximum,
  "minMaxLength" -> lintMinMaxLength,
...etc)

Then just get Linters by their keys from provided input:

val lintersToUse = lintersToSkip.foldLeft(Right(allLinters.values)) { (linters, cur) =>
  (linters, allLinters.get(cur)) match {
    case (Right(linters), Some(c)) => Right(linters.diff(List(c)))
    case (Left(errors), Some(_)) => Left(errors)
    case (Right(_), None) => Left(List(s"Unknown linter $cur"))
    case (Left(errors), None) => Left(s"Unknown linter $cur" :: errors)
  }
}

@chuwy
Copy link
Contributor Author

chuwy commented Jan 19, 2018

I'm also quite keen to have different sets of linters for different use cases, like redshift, config, etc. But not for this time.

@alexanderdean
Copy link
Member

I'm also quite keen to have different sets of linters for different use cases

That's a nice idea.

@oguzhanunlu oguzhanunlu reopened this Jan 23, 2018
@chuwy
Copy link
Contributor Author

chuwy commented Jan 25, 2018

Hey @oguzhanunlu, just a quick reminder to add description of all new linters Igluctl wiki page ("Update documenation" card in project this time is after you!)

@chuwy chuwy closed this as completed in f1eb904 Feb 7, 2018
rzats pushed a commit to snowplow/igluctl that referenced this issue Mar 22, 2019
rzats pushed a commit to snowplow/schema-ddl that referenced this issue Mar 25, 2019
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

No branches or pull requests

3 participants