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

Cleanup file structure, upper case Build.scala #273

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

tim-zh
Copy link
Contributor

@tim-zh tim-zh commented Oct 10, 2016

No description provided.

@tim-zh tim-zh changed the title Cleanup file structure, upper case Build.scala, #32 Cleanup file structure, upper case Build.scala Oct 10, 2016
@tim-zh
Copy link
Contributor Author

tim-zh commented Oct 10, 2016

There is a problem with producing a warning instead of an error when Build.scala is missing:
in that case BuildBuild.managedBuild has to return something, null or Some[BuildInterface]. Both options conflict with the rest of the code that relies on BuildBuild.managedBuild.

super(urls, parent);
assertExist(urls);
}
public CbtURLClassLoader(URL[] urls){
public CbtUrlClassLoader(URL[] urls){
Copy link
Owner

Choose a reason for hiding this comment

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

Let's try to keep commits focused on one thing please. Renaming build.scala into Build.scala is pretty large and throwing in changes like this make them very hard to discover. Can you break this up into separate commits please?

@cvogt
Copy link
Owner

cvogt commented Oct 10, 2016

regarding the warning: let's basically accept build.scala and Build.scala but show a warning by writing to System.err when build.scala is found. Since we'll accept it, we should have something to return, I believe?

@tim-zh
Copy link
Contributor Author

tim-zh commented Oct 11, 2016

The update takes care of #274.

@cvogt
Copy link
Owner

cvogt commented Oct 14, 2016

@tim-zh ah, I missed your updates. Just seeing them now. What do you think about using cbt-build instead of build.cbt? Might be easier to understand? Also see https://gitter.im/cvogt/cbt?at=58000253891a53016309c461

@@ -20,62 +21,70 @@ trait BuildBuild extends BaseBuild{

override def dependencies =
super.dependencies :+ context.cbtDependency
def managedBuildDirectory: java.io.File = lib.realpath( projectDirectory.parent )
def managedBuildDirectory: File = lib.realpath( projectDirectory.parent )
private object managedBuildCache extends Cache[BuildInterface]
def managedBuild = managedBuildCache{
val managedBuildFile = projectDirectory++"/Build.scala"
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of doing the below change, could we simple change this one val instead and show the warning if we are using build.scala here? Maybe easier than the nested if below.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed. I'll make it that way.

@cvogt cvogt added this to the 1.0 milestone Feb 21, 2017
@cvogt cvogt added cleanup and removed feature labels Mar 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants