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

Use Scala specific task markers. #330

Merged
merged 2 commits into from Feb 25, 2013

Conversation

dragos
Copy link
Member

@dragos dragos commented Feb 22, 2013

Don't use Java task markers for Scala sources, as the Java builder
removes them all on "full build", which is what we trigger from the
Sbt build manager if:

  • Java sources need recompilation
  • resources (non-source files) exist on the source path (so they are
    copied to the output folder)

This commit adds a Scala specific marker type (subtype of the Java one)
that preserves all functionality, but is only handled by our code.

Fixed #1001401, #1000975.

Don't use Java task markers for Scala sources, as the Java builder
removes them all on "full build", which is what we trigger from the
Sbt build manager if:

* Java sources need recompilation
* resources (non-source files) exist on the source path (so they are 
  copied to the output folder)

This commit adds a Scala specific marker type (subtype of the Java one)
that preserves all functionality, but is only handled by our code.

Fixed #1001401, #1000975.
@scala-jenkins
Copy link

Started jenkins job pr-validator-master-trunk at https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-trunk/435/

@scala-jenkins
Copy link

Started jenkins job pr-validator-master-2.9.x at https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-2.9.x/443/

@scala-jenkins
Copy link

jenkins job pr-validator-master-2.9.x: Success - https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-2.9.x/443/

@skyluc
Copy link
Member

skyluc commented Feb 22, 2013

LGTM

One thing though, FileUtils seems to contain a lot of unused imports. I would be great if you could clean it up.

@scala-jenkins
Copy link

jenkins job pr-validator-master-trunk: Success - https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-trunk/435/

@dragos
Copy link
Member Author

dragos commented Feb 22, 2013

Pushed another commit with organized imports.

@skyluc
Copy link
Member

skyluc commented Feb 22, 2013

Sweet. And still good for me.

@scala-jenkins
Copy link

Started jenkins job pr-validator-master-trunk at https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-trunk/436/

@scala-jenkins
Copy link

Started jenkins job pr-validator-master-2.9.x at https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-2.9.x/444/

@scala-jenkins
Copy link

jenkins job pr-validator-master-2.9.x: Success - https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-2.9.x/444/

@scala-jenkins
Copy link

jenkins job pr-validator-master-trunk: Success - https://jenkins.scala-ide.org:8496/jenkins/job/pr-validator-master-trunk/436/

@dragos
Copy link
Member Author

dragos commented Feb 22, 2013

Still waiting for a second pair of 👀

@dotta
Copy link
Member

dotta commented Feb 22, 2013

The patch looks good. Only comment I have is that we seem not to protect ourselves against "too big comments" that can cause assertion failures, see https://github.com/dragos/scala-ide/blob/907342a8655695fa59936689154eb63ac3a2b296/org.scala-ide.sdt.core/src/scala/tools/eclipse/resources/MarkerFactory.scala#L62

@dragos
Copy link
Member Author

dragos commented Feb 22, 2013

That's a good observation, but MarkerFactory as is assumes too much about the type of markers it creates (for instance, severity is required, but tasks don't have one). I have a bunch of other changes (like grouping all marker operations in a single workspaceRunnable) that I left out, as I wanted this PR to touch as little as possible. I can put this in that (future) PR.

dragos added a commit that referenced this pull request Feb 25, 2013
@dragos dragos merged commit f248290 into scala-ide:master Feb 25, 2013
dragos added a commit that referenced this pull request Feb 25, 2013
Use Scala specific task markers.(cherry picked from commit f248290)
@dragos
Copy link
Member Author

dragos commented Feb 25, 2013

Cherry-picked and pushed to 3.0.x (cherry-picked the merge commit, to test the changelog tool).

@dragos dragos deleted the isssue/tasks-disappear-1001401 branch February 25, 2013 10:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants