search

junkdog / artemis-odb

A continuation of the popular Artemis ECS framework
BSD 2-Clause "Simplified" License
776 stars 111 forks source link

Maven folder structure #4

Closed josephknight closed 10 years ago

josephknight commented 10 years ago

I would like to politely request that we not use the Maven folder structure as it adds unnecessary clutter. It would seem the previous folder structure can still be used by setting the src path as mentioned here:

http://stackoverflow.com/questions/4040888/maven-directory-structure

Just my two cents. I found this change disturbing and wanted express this politely. Thanks for everything, JosephKnight.com

josephknight commented 10 years ago

Also, perhaps a better reason, is that the source files still refer to non-maven paths (in their package declaration). I'm not sure what to do a new or existing Eclipse project in order to remedy this, but one thing is for sure: before this change all the public had to do was a simple "Create new Java Project" and point to artemis-odb.

Let's not change something from simple and elegant to more complex, further complicating setup, just because one tool (that many don't use) offers this folder convention as a changeable default.

lopho commented 10 years ago

I would also prefer having the source completely IDE/Maven/[Insert you thing here] agnostic.

junkdog commented 10 years ago

Hmm... yes, removing the eclipse project files from the repo was probably a bad idea. Sorry about that.

The sourceDirectory tag was actually used before, but after adding a unit test (under src/test/java), I changed the folder layout to maven's default. There might be a tag for setting alternate test source folders too, but I haven't seen it.

Personally, I prefer adhering to maven conventions whenever there is a pom.xml present. Naturally, it should still be easy working with the project from any IDE.

I'm not sure what level of penetration other IDE:s have, but Netbeans natively supports maven, intellij has good maven integration out of the box, jdeveloper supposedly plays nicely with maven too. Of course, this leaves Eclipse - which admittedly has some really flaky maven wiring.

So... I've created a new branch, fixing-eclipse and recreated the eclipse project files with mvn eclipse:eclipse, which lets Eclipse handle it as any other eclipse project. Unfortunately, there's a reference to junit, which it tries to locate inside the local maven repository.

I see two options:

  1. Remove src/test/java as a source folder under Eclipse - along with the junit classpath entry. The upside is that it works cleanly with Eclipse, but the unit tests won't run without manually setting it up inside eclipse.
  2. Leave it as it is, requiring users who don't have maven installed to manually link the junit lib. This is how the fixing-eclipse branch works atm.

Let me know if you have any ideas for improvements.

JosephKnight.com

Whoa, sweet art!

josephknight commented 10 years ago

Whoa, sweet art!

First of all, thanks for that. Compliments drive me.

I think the re-inclusion of Eclipse files fixed my package path issues but we now have the problem that you're enforcing the use of the unit test packages. Now we are required to include org.junit stuff and this is breaking our projects and complicating quick adoption of artemis into a noob's project. Shouldn't this unit test be outside the scope of the main artemis project?

I really want to follow this repo for my Artemis needs, but things are getting quite messy today. I don't want to leave but these complexities are consuming my dev time.

I have respect for your timely updates to Artemis in response to bugs the community reports on. This made your repo the best and if I end up using another will miss this greatly. I hope you'll reconsider these additions that moved this repo away from being so clean and pure as it was just a day ago.

Again, with regret and respect, Joseph Knight

junkdog commented 10 years ago

Ah, yeah, it struck me after pushing that it would probably have been the better option to exclude the test folder from the eclipse files - and with it the junit classpath entry. Anyone who wants to run the tests can still easily configure the environment or just run it from maven or IDE-of-choice.

I have respect for your timely updates to Artemis in response to bugs the community reports on. This made your repo the best and if I end up using another will miss this greatly. I hope you'll reconsider these additions that moved this repo away from being so clean and pure as it was just a day ago.

Hey, thanks man! I'll do my best to preserve her purity :) Hopefully this works out to everyone's satisfaction, If so I'm merging the branch back onto master.

lopho commented 10 years ago

I appreciate this! Thank you. Some perfomance enhancements and documentation of Bag/ImmutableBag are coming up soon!

junkdog commented 10 years ago

Cheers - and no problem!

I merged fixing-eclipse onto master.

junkdog commented 10 years ago

Oh yeah. I'm closing this issue now. Feel free to re-open it if you face any more problems.

josephknight commented 10 years ago

Ahhhh. Refreshing.

lopho commented 10 years ago

So, the maven directory structure will stay? Test folder as well? I just want to be sure, as I want to avoid rebasing twice before merging my bag-performance branch.

I hope I'm not rushing or anything. (o_o;)

junkdog commented 10 years ago

Yeah, the folder structure stays, but there shouldn't be any more interferences.

The test folder is not referenced by eclipse's project files, to avoid requiring non-maven eclipse users to manually add junit to the classpath.

Looking forward to the pull request But shouldn't it be possible for you to fetch my master branch and merge it into your own? Without rebasing? Maybe this is common knowledge to others, but I only last week found out about https://help.github.com/articles/syncing-a-fork - I could be wrong, but I think git is intelligent enough to resolve the differences.