> On Sept. 10, 2012, 6:47 p.m., Mike Percy wrote: > > I'm not sure this is a great idea from a maintainability perspective. > > Brock, do you think this is necessary?
Why is it a problem from a maintainability perspective? If someone changes the .proto file they need to regenerate the .java file and check the changes in. That is a rare event and they'd need to actually generate the source so they can use the change. The protoc compiler is old on the jenkins build slaves so it generates code which does not compile with recent versions of the protocol buffers java library. You can see the build failing here https://builds.apache.org/job/flume-trunk/296/console (portion of the message below). {noformat} [ERROR] /home/jenkins/jenkins-slave/workspace/flume-trunk/flume-ng-channels/flume-file-channel/target/generated-sources/java/org/apache/flume/chanel/file/proto/ProtosFactory.java:[11,22] org.apache.flume.chanel.file.proto.ProtosFactory.Checkpoint is not abstract and does not override abstract method newBuilderForType(com.google.protobuf.GeneratedMessage.BuilderParent) in com.google.protobuf.GeneratedMessage {noformat} - Brock ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/6985/#review11264 ----------------------------------------------------------- On Sept. 10, 2012, 4:37 p.m., Brock Noland wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/6985/ > ----------------------------------------------------------- > > (Updated Sept. 10, 2012, 4:37 p.m.) > > > Review request for Flume. > > > Description > ------- > > Instead of generating ProtoFactory each time we run maven, the class can be > generated using: > > mvn generate-sources -Pcompile-proto > > This is because we cannot depend on user/build machines having an updated > protoc compiler. > > > Diffs > ----- > > flume-ng-channels/flume-file-channel/pom.xml 639195e > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Commit.java > 7d57bb8 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/EventQueueBackingStoreFileV3.java > c766d09 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/LogFileV3.java > 414d6d9 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Put.java > 884ebde > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Rollback.java > 1a8ddff > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/Take.java > cfbe0c9 > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/TransactionEventRecord.java > f78147f > > flume-ng-channels/flume-file-channel/src/main/java/org/apache/flume/channel/file/proto/ProtosFactory.java > PRE-CREATION > flume-ng-channels/flume-file-channel/src/main/proto/filechannel.proto > 59dc8db > > Diff: https://reviews.apache.org/r/6985/diff/ > > > Testing > ------- > > unit tests pass > > > Thanks, > > Brock Noland > >
