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

Add folder syncing #27

Merged
merged 12 commits into from
Nov 19, 2014
Merged

Add folder syncing #27

merged 12 commits into from
Nov 19, 2014

Conversation

timowest
Copy link
Member

Fixes #26

copyIfChanged(file, file2);
} else {
file2.mkdir();
syncFiles(file, file2);
Copy link
Member

Choose a reason for hiding this comment

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

Because you're recursing here, would it be reasonable to add a test case that reflects a structure of directories with files?

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. I added a simple test case for it.

Take asynchronous compilation execution into account
@luisfpg
Copy link

luisfpg commented Nov 4, 2014

Another interesting thing on the file sync is create a temp directory inside the project target folder, not on the system temp.
It is common on Linux setups to have a /home partition separated from / (root) partition.
I think it would be faster to, when having to copy the file because it has been modified, instead of copying over, removing the old then attempting to rename, or else move. Bear in mind that rename fails if files are in distinct partitions.
This way all files should always be in the same partition, and only operations over file system metadata would be performed, not physical copies.

@Shredder121
Copy link
Member

I think that limiting ourselves to the confines of the target directory is better yes.

Currently, we generate a temp directory, which we don't clean up.
if we limit ourselves to just the target directory, inside let's say incremQBuild, we can guarantee the files are on the same filesystem and easily clean up.

@timowest
Copy link
Member Author

@luisfpg Are there still issues with the implementation?

@luisfpg
Copy link

luisfpg commented Nov 17, 2014

@timowest No more issues - everything I tested worked as expected.
What I've tested (in Eclipse):

  • Create a new entity -> the Q class is generated
  • Change the entity -> the Q class is updated
  • Remove the entity -> the Q class is removed

Congratulations!

@timowest
Copy link
Member Author

@Shredder121 Is this good to merge?

@Shredder121
Copy link
Member

Aside from maybe cleaning up the directory this looks fine yes.
Very nicely done indeed.

@timowest
Copy link
Member Author

Which directory needs to be ckeaned up?
On 19 Nov 2014 00:51, "Ruben Dijkstra" notifications@github.com wrote:

Aside from maybe cleaning up the directory this looks fine yes.
Very nicely done indeed.


Reply to this email directly or view it on GitHub
#27 (comment)
.

@Shredder121
Copy link
Member

Nevermind, only in the test case (FileSyncTest) the directory is not cleaned up.
The processor does clean up, my bad.

This is good to merge

note that I can't merge, as I don't have write access in this repository, so you'll have to do it

timowest added a commit that referenced this pull request Nov 19, 2014
@timowest timowest merged commit e04068e into master Nov 19, 2014
@timowest timowest deleted the i26 branch November 19, 2014 16:08
@timowest timowest removed the progress label Nov 19, 2014
@timowest
Copy link
Member Author

@Shredder121 I added you now to the contributors of all querydsl org repositories.

@Shredder121
Copy link
Member

AH, so I see, yes. Thank you very much.
This makes collaborating much easier.

@Shredder121
Copy link
Member

And then we have to update the docs again using 1.1.3?

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

Successfully merging this pull request may close these issues.

Performance hit / hot redeploy problem in Eclipse m2e apt generation
3 participants