I just did a quick search of thrift generated tool. It looks like there is no across system platform tools to do the job. We still need to install the thrift binary before generate the thrift related code. In this case I think it's fine to check in the generated code there. But we still need to update the rat[1] tool to skip the License header checking for those codes.
[1]https://creadur.apache.org/rat/ Willem Jiang Twitter: willemjiang Weibo: 姜宁willem On Wed, Dec 12, 2018 at 8:58 AM Xiangdong Huang <saint...@gmail.com> wrote: > > Hi, > > Christofer, thank you so much for your detailed suggestion. It is a good > chance to let us know how a clear and canonical maven project is. > > I have organized all of these issues and open 3 issues at Github: #502, > #503, #504 > > * https://github.com/thulab/iotdb/issues/502 > * https://github.com/thulab/iotdb/issues/503 > * https://github.com/thulab/iotdb/issues/504 > > And, all guys (current committers), it is time to fight. > > As for the release-profile, I have no experience about that. We will try > our best to fix it, and may ask for more help. :D > > As for the generated codes, especially for Thrfit, I have some question. > Generating Thrift code on Windows is easy, you just need a thrfit.exe file. > But for Linux and Mac OS, you have to install the thrift yourself. And you > must make sure that the version of the thrift is the same with us (v0.9). > If someone does not install thrift, Maven will says that something like > `/usr/local/bin/thrift does not exist`, and the maven build will failed. > Besides, we can find some other projects also check in the generated thrift > code, like Apache Cassandra v2.0[1]. > > Reference: > [1] https://github.com/apache/cassandra/tree/cassandra-2.0/interface > > Best, > ----------------------------------- > Xiangdong Huang > School of Software, Tsinghua University > > 黄向东 > 清华大学 软件学院 > > > Christofer Dutz <christofer.d...@c-ware.de> 于2018年12月12日周三 上午4:49写道： > > > > Hi all, > > > > So I was curious, checked out the code and ran a build. I had to disable > one test (more details down the document), but in the end I got a > successful build. Congrats on that … some of us – especially one other of > your mentors - know at least one project where it takes a little more > setting up to get it to build ;-) > > > > Here comes a list of things we will need changing for Apache (And or in > order to avoid a lot of problems in the future): > > > > > > * The parent pom in the root has to be set to “org.apache:apache:21” > (or the latest version of that) > > * As far as I know the ASF doesn’t like the “developers” section in > the pom files > > * All non-binary files need the ASF header (Yes … all of them … I > know this will not be the most glorious job, but it has to be done) > > * Module Grafana > > * The parent of this module is not within the project scope. This > quite often causes problems with the maven tooling and when releasing, the > module it will not inherit the settings of the apache parent. This has to > be changed (Try adding the iotdb-parent as parent and adding the missing > configuration options from the spring-boot-starter-parent. This way you > don’t have to redefine all the settings you can no longer inherit from the > iotdb-parent. > > * Module service-rpc and tsfile: > > * You are defining a “release” profile which is obviously intended > for releasing. This will not apply as apache releases are performed with > the “apache-release” profile. So if important stuff is to be done here (it > actually shouldn’t) > > * From a quick look all defined there is also done by the > profile defined in the “apache-release” profile in the “apache” parent pom. > So all except the exclusion for package names including “thrift” could > simply be removed. (Maybe it’s worth configuring the maven-javadoc-plugin > in a pluginManagement section of the root to exclude them in general and > get rid of the “release” profiles in general) > > * Especially the staging part has to be removed as you will be > staging to repository.apache.org instead of the oss sonatype repo. > > > > Things I personally would suggest to change: > > > > > > * Don’t use a property to define the version of the artifacts that > are part of the build (project.version) (The release plugin updates this > for you and believe me I have encountered a lot of problems with this > approach in my last 10 years of managing Maven projects as a consultant) > > * Why are you generating code (thrift) and checking that in? I would > strongly suggest to always generate the code, move the thrift files to > “src/main/thrift” and have the build generate things ot its usual location > “target/generated-sources/thrift” (or something like that … the last part > of the path may differ) > > * General observations: > > * Replace the antrun plugin with the dependencies plugin for > copying the libs to the iotdb/lib directory (antrun is highly deprecated) > > * No need to re-define the license, scm, distribution management > in sub-modules > > * Try to unify the plugin definitions in the root pom. > > * The “release” profile could be causing problems in a real > release (will not be executed as with apache it’s called “apache-release”) > > * You are relying on Java 8 so the Jodatime library is included in > general there is no need to use the external library and the included > classes could be used) > > * It might we worth defining an “example” module which contains > real maven buildable examples (some projects even move them to a separate > examples repository) right now there is no way to automatically detect that > the code has changed and the example no longer matches that API. > > * I wouldn’t produce fat jars (using the assembly plugin) as this > bundles classes in to a library that might be used in another application > that has dependencies to the same libraries but in a different version … > trust me … these are the worst problems to track down. > > * Module iotdb: > > * Have the MANIFEST.MF generated by maven instead. > > * You have a dependency on the antlr3 maven plugin. Instead you > should depend on the antlr-runtime artifact that matches your antlr3 > version. > > * What is the “kvmatch-iotdb” artifact referring to? Seems it is > not part of this project. > > * Why are you manually configuring an additional compilation in > the pre-integration-test phase? There is no code in “src/it/java” > > * One test is failing because I’m in a different time zone: Failed > tests: test(cn.edu.tsinghua.iotdb.sql.DateFormatTest): > expected:<…02-01T11:12:35.000+0[1]:00> but > was:<…02-01T11:12:35.000+0[8]:00> > (cn.edu.tsinghua.iotdb.sql.DateFormatTest.test has most references ending > with “+08:00” which makes the tests only run in one time-zone) > > * > cn.edu.tsinghua.iotdb.queryV2.engine.control.OverflowFileStreamManager uses > the deprecated internal SUN API which is not available in Java 11 anymore > > * Module jdbc: > > * You are adding a “interface/thrift” directory to the sources, > but that doesn’t exist. > > * Module service-rpc: > > * Move the interfaces thirft files to “src/main/thrift” and have > the code generated on every build to “target/generated-sources” (default) > > * Don’t check in generated code. > > * Module tsfile_ > > * In general the same remarks apply as to the service-rpc module > > * Module spark > > * You have included *.tsfile in your “.gitglobal” but seem to have > forced in a “tsfile”-file in the test resources (Things can get quite messy > in such situations if the file is ever changed, as the IDE will never > detect changes and never commit any changes to that file) > > * Module Hadoop > > * Looks a little unfinished … so I won’t go into details here. > > > > So much for a quick run through the project … If you want, I could be of > assistance with the actual transformation … I know I could probably address > most in half a day and that most people would probably take a lot longer. > After all we want you to learn the Apache way … not to learn setting up > projects ;-) (Other mentors please correct me if I’m wrong) > > > > So … I’ll call it a night or my girlfriend will get angry ;-) > > > > Chris > > > >
