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

review feat: API for incremental build #2051

Closed
wants to merge 13 commits into from

Conversation

surli
Copy link
Collaborator

@surli surli commented Jun 11, 2018

This PR aims at finishing the incremental build mechanism proposed by @Egor18 in #1905 with an API well integrated with Spoon Environment and standard launcher.

@surli
Copy link
Collaborator Author

surli commented Jun 11, 2018

@Egor18 WDYT about this API? It's not polished yet: I'm thinking about removing the new fields from Launcher that seem unecessary and to put them as local variable, do you agree?

@Egor18
Copy link
Contributor

Egor18 commented Jun 11, 2018

This API looks good, thanks. Yes, unecessary fields can be removed.
Also, probably we need some perfomance tests.
And finally, I'm not sure if comparison of the modification time of the files is really sufficient for incremental build. What should we do if someone just copies another file?

@surli
Copy link
Collaborator Author

surli commented Jun 11, 2018

ok I'll try first to improve it without changing the way it works.

We'll be able to improve the way we detect changes later: for now modification time can already be used in several cases.

@surli surli changed the title feat: API for incremental build review feat: API for incremental build Jun 12, 2018
@spoon-bot
Copy link
Collaborator

API changes: 4 (Detected by Revapi)

Old API: fr.inria.gforge.spoon:spoon-core:jar:6.3.0-20180611.225429-136 / New API: fr.inria.gforge.spoon:spoon-core:jar:6.3.0-SNAPSHOT

Method was added to an interface.
Old none
New method Environment#getCacheDirectory()
Breaking binary: non_breaking
Method was added to an interface.
Old none
New method Environment#isIncremental()
Breaking binary: non_breaking
Method was added to an interface.
Old none
New method Environment#setCacheDirectory(File)
Breaking binary: non_breaking
Method was added to an interface.
Old none
New method Environment#setIncremental(boolean)
Breaking binary: non_breaking

/**
* This class store information for incremental build.
*/
public class IncrementalBuildInformation {
Copy link
Collaborator

Choose a reason for hiding this comment

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

put @Internal or in package called something.internal (eg spoon.launcher.internal)

@monperrus
Copy link
Collaborator

Incremental launcher is a different responsibility. What about having an IncrementalLauncher for this? (which would delegate to Launcher)

@surli
Copy link
Collaborator Author

surli commented Jun 12, 2018

So actually the incremental launcher was really the API provided by @Egor18 in #1905 Maybe his original PR is enough then?

@monperrus
Copy link
Collaborator

Why was #1905 still marked as WIP?

@surli
Copy link
Collaborator Author

surli commented Jun 13, 2018

Why was #1905 still marked as WIP?

I suppose because I asked for changes regarding the API...

@monperrus
Copy link
Collaborator

:-)

did you add new tests here?

@surli
Copy link
Collaborator Author

surli commented Jun 13, 2018

No

@monperrus
Copy link
Collaborator

Thanks a lot @surli to have revived this topic!

@monperrus monperrus closed this Jun 13, 2018
@monperrus
Copy link
Collaborator

That will be a great release!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants