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

Fix #1050: Fix one known and some potential setting name conflicts. #1053

Merged
merged 1 commit into from Sep 16, 2014

Conversation

Projects
None yet
5 participants
@sjrd
Copy link
Member

sjrd commented Sep 16, 2014

Since sbt identifies settings and tasks by name, they must be
unique globally amonst all plugins in the world. This commit
renames some of the tasks and settings that would most likely
cause troubles while not being used much in builds.

@sjrd

This comment has been minimized.

Copy link
Member

sjrd commented Sep 16, 2014

Review by @gzm0
/cc @japgolly

Fix #1050: Fix one known and some potential setting name conflicts.
Since sbt identifies settings and tasks by name, they must be
unique globally amonst all plugins in the world. This commit
renames some of the tasks and settings that would most likely
cause troubles while not being used much in builds.
@vendethiel

This comment has been minimized.

Copy link

vendethiel commented Sep 16, 2014

Isn't there a way just to have scalajs: as a prefix (in the console tasks) and then, every other setting (in the SBT file) under scalaJSKeys?

@sjrd

This comment has been minimized.

Copy link
Member

sjrd commented Sep 16, 2014

@Nami-Doc No, there isn't.

@scala-jenkins

This comment has been minimized.

Copy link

scala-jenkins commented Sep 16, 2014

Test PASSed.
Refer to this link for build results: https://scala-webapps.epfl.ch/jenkins/job/scalajs-pr/506/

@gzm0

This comment has been minimized.

Copy link
Contributor

gzm0 commented Sep 16, 2014

LGTM

gzm0 added a commit that referenced this pull request Sep 16, 2014

Merge pull request #1053 from sjrd/prefix-setting-names
Fix #1050: Fix one known and some potential setting name conflicts.

@gzm0 gzm0 merged commit 4a18cdf into scala-js:master Sep 16, 2014

1 check passed

default Merged build finished.
Details

@sjrd sjrd deleted the sjrd:prefix-setting-names branch Sep 16, 2014

@japgolly

This comment has been minimized.

Copy link
Contributor

japgolly commented Sep 16, 2014

Nice work! Unfortunate that it's required but that's life with SBT I guess.

Regarding @Nami-Doc 's question about a scalajs: prefix, are you sure that it's not possible? xsbt-web-plugin 0.9 does a similar things where it has tasks and settings prefixed by container:. It doesn't have the relationship with the regular SBT phases that Scala.js does but on the off-chance that there's some helpful trick or technique in there that you guys aren't aware of, I thought I'd mention it.

@scala-jenkins

This comment has been minimized.

Copy link

scala-jenkins commented Sep 16, 2014

Test PASSed.
Refer to this link for build results: https://scala-webapps.epfl.ch/jenkins/job/scalajs-pr/509/

@sjrd

This comment has been minimized.

Copy link
Member

sjrd commented Sep 16, 2014

The container: prefix is actually a configuration in sbt's terminology. xsbt-web-plugin introduces its own configuration for its stuff, hence the prefix. Scala.js integrates deeply with the normal compile and test configurations, and that's what makes it "just work" out of the box with things like packaging, dependencies, publishing, and all kinds of stuff. You don't want Scala.js to have its own configurations, trust me.

Besides, that wouldn't help with this issue. Configurations do not really namespace keys (they're not prefixes). They scope them. But every (non-scoped) key name must still be unique.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment