-
Notifications
You must be signed in to change notification settings - Fork 935
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
Add warning for unknown configurations #4231
Conversation
@steinybot Thanks for the contribution. Looks like a lot of work went into this. I am a bit concerned about the the failure on AppVeyor however:
This kind of looks related, but I am not sure why it's passing on Travis if there's an issue. |
log.warn( | ||
s"""The project $project references an unknown configuration "$config" and was guessed to be "$guess".""" | ||
) | ||
log.warn("This configuration should be explicitly added to the project.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Never mind. I think restarting the AppVeyor job fixed it, so I guess that's an unrelated issue. |
We made a survey form to collect feedback on the contribution process. Please fill it out when get you get a chance. https://goo.gl/forms/LDPlTl9Wso7YwEri2 |
@@ -383,8 +387,8 @@ trait ParserMain { | |||
} | |||
|
|||
/** Presents a Char range as a Parser. A single Char is parsed only if it is in the given range.*/ | |||
implicit def range(r: collection.immutable.NumericRange[Char]): Parser[Char] = | |||
charClass(r contains _).examples(r.map(_.toString): _*) | |||
implicit def range(r: collection.immutable.NumericRange[Char], label: String): Parser[Char] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just noticed bincompat breakages in this PR now. Even though Parser is currently under "internal" package, it's publicly used so I don't think we can break bincompat.
@@ -396,7 +400,7 @@ trait ParserMain { | |||
* Defines a Parser that parses a single character only if the predicate `f` returns true for that character. | |||
* If this parser fails, `label` is used as the failure message. | |||
*/ | |||
def charClass(f: Char => Boolean, label: String = "<unspecified>"): Parser[Char] = | |||
def charClass(f: Char => Boolean, label: String): Parser[Char] = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This breaks source compat.
An unfortunate consequence of this is that you will get a spurious warning from the implicit root project any time you have e.g. an // Entire contents of a build.sbt
inThisBuild(
fork in Test := true
)
|
@ches Let's call that a regression. Could you open an issue for it? |
Just opened sbt/sbt-multi-jvm#48 regarding:
I am not sure at this point if this warning is really meaningful to most build users. |
sbt 1.2.0 changed the prototype of Load.reapply(), so we use reflection to call the proper version; this commit restores sbt 1.2.0 compatibility. Fixes #212 Ref sbt/sbt#4231
Fixes sbt#4293 Ref sbt#4231, sbt#4065 This fixes the regression on sbt 1.2.0 that displays a lot of warnings about configurations. The warning was added in sbt#4231 in an attempt to fix sbt#4065. This actually highlights somewhat loose usage of configurations among the builds in the wild, and the limitation on the current slash syntax implementation. I think we can remove this warning for now, and try to fix sbt#4065 by making slash syntax more robust. In particular, we need to memorize the mapping between the configuration name and Scala identifier across the entire build, and use that in the shell.
Fixes sbt#4293 Ref sbt#4231, sbt#4065 This fixes the regression on sbt 1.2.0 that displays a lot of warnings about configurations. The warning was added in sbt#4231 in an attempt to fix sbt#4065. This actually highlights somewhat loose usage of configurations among the builds in the wild, and the limitation on the current slash syntax implementation. I think we can remove this warning for now, and try to fix sbt#4065 by making slash syntax more robust. In particular, we need to memorize the mapping between the configuration name and Scala identifier across the entire build, and use that in the shell.
Adds a warning for unknown configurations. This helps to mitigate the issue in #4065 where the id's of unknown configurations are guessed incorrectly and cannot be used with the new unified
/
scope syntax.