facebookarchive / swift

An annotation-based Java library for creating Thrift serializable types and services.
Apache License 2.0
900 stars 297 forks source link

Generated comment #328

Closed SergeyMakarenko closed 7 years ago

SergeyMakarenko commented 7 years ago

This change adds comment like this to each swift generated java file: // Generated by swift-generator from /Users/smakarenko/src/swift/swift-generator/target/test-classes/basic/common/fb303/if/fb303.thrift on Tue Oct 18 15:21:46 PDT 2016

electrum commented 7 years ago

This seems like a bad idea. Why would you want the generated files to be non deterministic and change even if the input is the same? This will break incremental build systems.

How about using a relative path and a checksum of the input file?

SergeyMakarenko commented 7 years ago

@electrum I changed path in generated file's comment to be relative. I'm actually not sure if putting date is a good idea I just used antlr's code generator as example where date is present. We can remove date and just leave relative path. Also I'm not sure how one could use checksum of the input file. Regarding incremental build breakage. I would assume that if idl file was not changed generation phase should not be triggered. Could you explain more how this could break? Thanks.

sazonovkirill commented 7 years ago

@electrum You mentioned that non-deterministic will break incremental build system. Did you mean in general or is there a particular incremental build system that this change will break?

@SergeyMakarenko I missed that, but I think date time of generation is not necessary. I'd rather have rev number, but that also have the downside of being non-deterministic. I suggest for now we just skip this part, let's just leave path to file (without timestamp or rev) and move on.

dain commented 7 years ago

@sazonovkirill it will break the Maven incremental build system, and in general breaks any incremental system that is not aware of our custom plugin.

SergeyMakarenko commented 7 years ago

Removed timestamp from generated comment