search

querydsl / apt-maven-plugin

Maven APT plugin
Apache License 2.0
79 stars 41 forks source link

Add folder syncing #27

Closed timowest closed 9 years ago

timowest commented 9 years ago

Fixes #26

luisfpg commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@luisfpg Are there still issues with the implementation?

luisfpg commented 9 years ago

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

Congratulations!

timowest commented 9 years ago

@Shredder121 Is this good to merge?

Shredder121 commented 9 years ago

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

timowest commented 9 years ago

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 https://github.com/querydsl/apt-maven-plugin/pull/27#issuecomment-63560905 .

Shredder121 commented 9 years ago

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 commented 9 years ago

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

Shredder121 commented 9 years ago

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

Shredder121 commented 9 years ago

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