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

Move common settings to an autoplugin #409

Merged
merged 1 commit into from Oct 19, 2016
Merged

Conversation

2m
Copy link
Member

@2m 2m commented Oct 19, 2016

Fixes #408

inThisBuild sets project scope to the current build, but settings set in the /build.sbt are added to the root project only.

Instead moved common settings to an AutoPlugin that is applied to every project.

@akka-ci akka-ci added the validating PR that is currently being validated by Jenkins label Oct 19, 2016
@akka-ci
Copy link

akka-ci commented Oct 19, 2016

Test PASSed.

@akka-ci akka-ci added tested PR that was successfully built and tested by Jenkins and removed validating PR that is currently being validated by Jenkins labels Oct 19, 2016
@ktoso
Copy link
Member

ktoso commented Oct 19, 2016

LGTM, thanks for the fix. Very weird that this is how inThisBuild works there... Oh well

@ktoso ktoso merged commit 26673ee into akka:master Oct 19, 2016
@ktoso
Copy link
Member

ktoso commented Oct 19, 2016

I'll release RC1 again, so it has the proper version suffix.

@jrudolph
Copy link
Member

I don't think this was necessary. We set crossPaths := false in Publish.scala which is what disables the version suffixes.

@ktoso
Copy link
Member

ktoso commented Oct 19, 2016

Why did we have crossPaths false anywhere in the first place...? Weird.
I think you're right.

I published the correct artifacts now (@2m's patch also works, but we should remove that crossPaths := false I think)

@jrudolph
Copy link
Member

Yes, we need to remove that. Otherwise, we might rely on the ordering of loading plugins.

ktoso added a commit that referenced this pull request Oct 19, 2016
@ktoso ktoso mentioned this pull request Oct 19, 2016
@2m 2m deleted the wip-408-common-settings branch October 19, 2016 10:24
ktoso added a commit that referenced this pull request Oct 19, 2016
* Revert "Move common settings to an autoplugin #408 (#409)"

This reverts commit 26673ee.

* +build include crossVersion in artifact names
@2m
Copy link
Member Author

2m commented Oct 19, 2016

crossPaths := false makes sure that there is only one target directory even when cross building. It is required during the release process. So everything ends up in the same directory, which then can be uploaded to S3.

@ktoso
Copy link
Member

ktoso commented Oct 19, 2016

S3 upload what? We don't do a distribution here hm.

@2m
Copy link
Member Author

2m commented Oct 19, 2016

Yes, you are right. Akka Http does not publish to S3. Akka does. And that is where crossPaths := false came from. So from release point of view having crossPaths := false is fine.

@ktoso
Copy link
Member

ktoso commented Oct 19, 2016

Uhm, you mean removing it is fine, since we don't use it - we don't have a "release" (zip) here,

ktoso added a commit that referenced this pull request Oct 25, 2016
ktoso added a commit that referenced this pull request Oct 25, 2016
* Revert "Move common settings to an autoplugin #408 (#409)"

This reverts commit 26673ee.

* Make performance specs run again (on jenkins too)
jedrekn pushed a commit to jedrekn/akka-http that referenced this pull request Nov 17, 2016
jedrekn pushed a commit to jedrekn/akka-http that referenced this pull request Nov 17, 2016
* Revert "Move common settings to an autoplugin akka#408 (akka#409)"

This reverts commit 26673ee.

* +build include crossVersion in artifact names
jedrekn pushed a commit to jedrekn/akka-http that referenced this pull request Nov 17, 2016
* Revert "Move common settings to an autoplugin akka#408 (akka#409)"

This reverts commit 26673ee.

* Make performance specs run again (on jenkins too)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tested PR that was successfully built and tested by Jenkins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants