Skip to content
This repository has been archived by the owner on Sep 12, 2021. It is now read-only.

split silhoutte in two projects #256

Merged
merged 1 commit into from Jan 16, 2015
Merged

Conversation

rfranco
Copy link
Contributor

@rfranco rfranco commented Jan 12, 2015

Here is the initial idea to split silhoutte and testkit

@akkie
Copy link
Contributor

akkie commented Jan 12, 2015

Maybe we should move the silhouette and silhouette-testkit directories into a top level src directory. We could also rename the app directories into main. What do you think?

@rfranco
Copy link
Contributor Author

rfranco commented Jan 12, 2015

I followed spray and akka style for directory structure.
https://github.com/akka/akka
https://github.com/spray/spray

About app folder, it's a play project for that reason use that strucutre.
We can use a normal sbt project so src folder will be
src
. main
.. resources
.. scala
. test
.. resources
.. scala

What do you perfer?

@akkie
Copy link
Contributor

akkie commented Jan 12, 2015

I would follow the play structure without the additional src inside the project directory. The scala directory isn't needed, because we have only a Scala API at this time.

Sure It's a Play project but it's rather a lib as an app. So I think main is OK.

So I would prefer:

src
  silhouette
    conf
    main
    test
  silhouette-testkit
    main
    test

Thoughts?

@rfranco
Copy link
Contributor Author

rfranco commented Jan 12, 2015

I think it should follow play or sbt directory structure (http://www.scala-sbt.org/0.13/tutorial/Directories.html)
because to change that we need to customize sbt config and i always prefer to follow the pattern.

I suggest to do the play schema without that src folder:

root 
. project (sbt config)
. silhouette
.. app
.. conf
.. test
. silhouette-testkit
.. app
.. conf
.. test

@akkie
Copy link
Contributor

akkie commented Jan 12, 2015

Ok, I'm fine with your suggestion. For me it was just a personal matter of taste.

@akkie
Copy link
Contributor

akkie commented Jan 14, 2015

Do you work on fixing the build errors?

@rfranco
Copy link
Contributor Author

rfranco commented Jan 14, 2015

I'm working on that but i'm a little busy in this week.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.22%) when pulling b0aa7c9 on rfranco:release/spit-testkit into d8e8547 on mohiva:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.66%) when pulling b0aa7c9 on rfranco:release/spit-testkit into d8e8547 on mohiva:master.

@akkie
Copy link
Contributor

akkie commented Jan 14, 2015

Looks good expect of the SilhouetteSpec. This file must be back into the silhouette project. What was the reason for this change?

@rfranco
Copy link
Contributor Author

rfranco commented Jan 14, 2015

because SilhouetteSpec use the testkit and if silhouette project depends on testkit that give recursive problem.

[error] /play-silhouette/project/Build.scala:8: recursive value testkit needs type
[error]   dependencies = Seq(testkit % "compile;test->test")
[error]                      ^
[error] one error found

Oneway is create other project silhouette-test and that depends on silhouette and testkit

@akkie
Copy link
Contributor

akkie commented Jan 14, 2015

Ahhh, I see if I can fix that. I'll then create a pull request to your branch.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 1f2aeec on rfranco:release/spit-testkit into d8e8547 on mohiva:master.

override def projectSettings = Seq(
organization := "com.mohiva",
version := "2.0-SNAPSHOT",
resolvers ++= Dependencies.resolvers,
Copy link
Contributor

Choose a reason for hiding this comment

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

@rfranco The resolver "Atlassian Releases" at "https://maven.atlassian.com/public/" must be the first because of this bug: https://groups.google.com/forum/#!topic/sbt-dev/EQVZGcYtBp0

@rfranco
Copy link
Contributor Author

rfranco commented Jan 16, 2015

@akkie I deleted from my local ivy and run again and it resolve.

[info] downloading https://maven.atlassian.com/public/com/atlassian/jwt/jwt-core/1.2.3/jwt-core-1.2.3.jar ...
[info]  [SUCCESSFUL ] com.atlassian.jwt#jwt-core;1.2.3!jwt-core.jar (1233ms)
[info] downloading https://maven.atlassian.com/public/com/atlassian/jwt/jwt-api/1.2.3/jwt-api-1.2.3.jar ...
[info]  [SUCCESSFUL ] com.atlassian.jwt#jwt-api;1.2.3!jwt-api.jar (625ms)
[info] Done updating.

@akkie
Copy link
Contributor

akkie commented Jan 16, 2015

I'm not the only one with this problem.

https://groups.google.com/forum/#!topic/play-silhouette/oSai5SgibLQ
https://github.com/mohiva/play-silhouette-seed/issues/20

And if I test it I get the error:

[info] downloading https://repo.typesafe.com/typesafe/releases/com/atlassian/jwt/jwt-core/1.2.3/jwt-core-1.2.3.jar ...
[warn]  [FAILED     ] com.atlassian.jwt#jwt-core;1.2.3!jwt-core.jar: The HTTP response code for https://repo.typesafe.com/typesafe/releases/com/atlassian/jwt/jwt-core/1.2.3/jwt-core-1.2.3.jar did not indicate a success. See log for more detail. (170ms)
[warn]  [FAILED     ] com.atlassian.jwt#jwt-core;1.2.3!jwt-core.jar: The HTTP response code for https://repo.typesafe.com/typesafe/releases/com/atlassian/jwt/jwt-core/1.2.3/jwt-core-1.2.3.jar did not indicate a success. See log for more detail. (170ms)
[warn] ==== Typesafe Releases Repository: tried
[warn]   https://repo.typesafe.com/typesafe/releases/com/atlassian/jwt/jwt-core/1.2.3/jwt-core-1.2.3.jar
[info] downloading https://repo.typesafe.com/typesafe/releases/com/atlassian/jwt/jwt-api/1.2.3/jwt-api-1.2.3.jar ...
[warn]  [FAILED     ] com.atlassian.jwt#jwt-api;1.2.3!jwt-api.jar: The HTTP response code for https://repo.typesafe.com/typesafe/releases/com/atlassian/jwt/jwt-api/1.2.3/jwt-api-1.2.3.jar did not indicate a success. See log for more detail. (180ms)
[warn]  [FAILED     ] com.atlassian.jwt#jwt-api;1.2.3!jwt-api.jar: The HTTP response code for https://repo.typesafe.com/typesafe/releases/com/atlassian/jwt/jwt-api/1.2.3/jwt-api-1.2.3.jar did not indicate a success. See log for more detail. (180ms)

Do you use sbt instead of activator?

@akkie
Copy link
Contributor

akkie commented Jan 16, 2015

Ahhh, I see. Do you mean that the Atlassian resolver is already the first in the list? My test isn't from Silhouette itself.

@rfranco
Copy link
Contributor Author

rfranco commented Jan 16, 2015

Yep, Atlassian is the first

> resolvers
[info] silhouette/*:resolvers
[info]  List(Atlassian Releases: https://maven.atlassian.com/public/, Typesafe Releases Repository: https://repo.typesafe.com/typesafe/releases/)
[info] silhouette-testkit/*:resolvers
[info]  List(Atlassian Releases: https://maven.atlassian.com/public/, Typesafe Releases Repository: https://repo.typesafe.com/typesafe/releases/)
[info] root/*:resolvers
[info]  List(Atlassian Releases: https://maven.atlassian.com/public/)

@akkie
Copy link
Contributor

akkie commented Jan 16, 2015

Nice! Please can you combine the commits into one commit. Then I can merge the pull request. Thanks.

@rfranco
Copy link
Contributor Author

rfranco commented Jan 16, 2015

Done! 👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) when pulling 2f0c8ff on rfranco:release/spit-testkit into d8e8547 on mohiva:master.

akkie added a commit that referenced this pull request Jan 16, 2015
@akkie akkie merged commit b469a30 into mohiva:master Jan 16, 2015
@akkie
Copy link
Contributor

akkie commented Jan 16, 2015

Thanks for your work on that!

@rfranco rfranco deleted the release/spit-testkit branch January 16, 2015 20:20
@coveralls
Copy link

Coverage Status

Coverage decreased (-5.6%) to 93.739% when pulling 2f0c8ff on rfranco:release/spit-testkit into d8e8547 on mohiva:master.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants