Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Upgrade to Maven 3 #174

Merged
merged 1 commit into from
Jul 23, 2014
Merged

Upgrade to Maven 3 #174

merged 1 commit into from
Jul 23, 2014

Conversation

benmccann
Copy link
Contributor

I didn't test this at all except to run mvn test. Do you think that's sufficient and likely to work?

@benmccann benmccann mentioned this pull request Jul 22, 2014
Closed
@muuki88 muuki88 mentioned this pull request Jul 22, 2014
Closed
@tcurdt
Copy link
Owner

tcurdt commented Jul 22, 2014

Quite a big change. Guess the next release should be 1.3 then.

@tcurdt
Copy link
Owner

tcurdt commented Jul 22, 2014

...and thanks for the pull request!

@tcurdt
Copy link
Owner

tcurdt commented Jul 22, 2014

@tmortagne @ebourg any objections to merge?

tmortagne
tmortagne reviewed Jul 22, 2014
View reviewed changes
pom.xml
</dependency>
<dependency>
<groupId>org.apache.ant</groupId>
<artifactId>ant</artifactId>
<version>1.9.3</version>
<version>1.9.4</version>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't see what this has to do with Maven 3.

@tmortagne
Copy link
Collaborator

I don't have anything against moving to Maven 3, keeping Maven 2 support does not really bring much.

Now on the pull request itself, there is several things that don't have anything to do with Maven so several commits would be cleaner IMO.

tmortagne
tmortagne reviewed Jul 22, 2014
View reviewed changes
pom.xml
@@ -256,7 +251,7 @@
<dependency>
<groupId>junit</groupId>
<artifactId>junit</artifactId>
<version>3.8.2</version>
<version>4.11</version>
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, I don't see the relationship with Maven.

@tcurdt
Copy link
Owner

tcurdt commented Jul 22, 2014

Indeed - it's more a "upgrade of dependencies" pull request like this.

@tcurdt
Copy link
Owner

tcurdt commented Jul 22, 2014

...but as long as it works I wouldn't be too anal about it.

@benmccann
Copy link
Contributor Author

Yeah, guess I went a little overboard :-) I took those off this pull request. Can do them separately in a later one perhaps

@tcurdt
Copy link
Owner

tcurdt commented Jul 22, 2014

Awesome!

@muuki88
Copy link
Contributor

muuki88 commented Jul 22, 2014

Awesome work @benmccann !

@tmortagne
Copy link
Collaborator

Thanks @benmccann :)

@ebourg
Copy link
Collaborator

ebourg commented Jul 22, 2014

Do we know the exact dependency that pulls the old version of slf4j?

@benmccann
Copy link
Contributor Author

Given #173, I would guess that it's maven-core

@tcurdt tcurdt added this to the 1.3 milestone Jul 23, 2014
@tcurdt
Copy link
Owner

tcurdt commented Jul 23, 2014

@ebourg you OK to install a cron job? because then I would say - let's merge that baby :)

@ebourg
Copy link
Collaborator

ebourg commented Jul 23, 2014

Go ahead, I'll stick with an older version until I get the time to configure Nexus.

tcurdt added a commit that referenced this pull request Jul 23, 2014
@tcurdt
Merge pull request #174 from benmccann/mvn3
45f3e53 
Upgrade to Maven 3
@tcurdt tcurdt merged commit 45f3e53 into tcurdt:master Jul 23, 2014
@muuki88
Copy link
Contributor

muuki88 commented Jul 23, 2014

Boom! When will the 1.3 release be?

@tcurdt
Copy link
Owner

tcurdt commented Jul 23, 2014

@muuki88 I'll be offline for 7 weeks from next Monday. No promises for a release before.

@benmccann
Copy link
Contributor Author

Thank you guys so much! @ebourg really, really appreciate your flexibility with the Maven version

@benmccann
Copy link
Contributor Author

Ay-ay-ay! 2 months is a long time! Is there anything we can help with to speed that up? Even a milestone or RC release would be helpful. It doesn't have to be a final version

@ebourg
Copy link
Collaborator

ebourg commented Jul 23, 2014

Well, install Nexus and deploy a snapshot? ;)

@muuki88
Copy link
Contributor

muuki88 commented Jul 23, 2014

No publish rights for org.vafer on maven central I'm afraid ;)

@benmccann benmccann mentioned this pull request Jul 23, 2014
Closed
@benmccann benmccann mentioned this pull request Jul 23, 2014
Closed
@tcurdt
Copy link
Owner

tcurdt commented Jul 23, 2014

@muuki88 I am sure he meant a local instance ;)

@benmccann We don't really do RC releases and I am sure everyone here knows how to build this little project.

The more people reporting 1.3-SNAPSHOT to be working fine the more likely I am to release it without testing it myself (which I don't have time for before I leave). It would not be so cool to have a broken 1.3 out while I am gone.

@benmccann
Copy link
Contributor Author

@tcurdt is there anything you'd do to test it yourself that isn't covered by the unit tests? Maybe we can do that testing for you

I can see how releasing a broken version and then leaving town would be not the most awesome situation. At the same time, I don't think anyone would be too upset given that 1.2 would still work. If you want to break from convention, I think an RC version would stop anyone from complaining if there were an issue

@tcurdt
Copy link
Owner

tcurdt commented Jul 23, 2014

Good question. I usually just do a couple of random builds of existing projects with it. No real testing plan :) Would be great if we had more tests for this.

Of course you are right about 1.2 still being around. As I am a friend of release early and often I might be OK with that.

@benmccann
Copy link
Contributor Author

+1 to release early and often! =)

i just tested this with sbt-native-packager and it worked well

jdeb$ mvn install

Add to resolvers += "Local Maven Repository" at "file:///" + Path.userHome + "/.m2/repository" to sbt-native-packager's build.sbt change to jdeb dependency to "1.3-SNAPSHOT"

Then run:

$ sudo apt-get install dpkg-dev
$ sbt scripted

All the tests pass:

Running debian / sysvinit-deb
Running debian / log-directory
Running debian / test-mapping-helpers
Running debian / test-mapping
Running debian / daemon-user-deb
Running debian / upstart-deb
Running debian / daemon-user-shell-deb
Running debian / override-control-files
Running debian / simple-jdeb
Running debian / java-app-archetype
Running debian / systemd-deb
Running debian / gen-changes
Running debian / simple-deb

@tcurdt
Copy link
Owner

tcurdt commented Jul 23, 2014

oh you have some external tests as well. that's cool.

@benmccann
Copy link
Contributor Author

Yeah, sbt-native-packager has a pretty big suite of integration tests. The tests above are all integration tests. There's another suite of unit tests that's also run separately. I feel pretty good about the changes given that they passed both the jdeb and sbt-native-packager tests. That should be pretty good coverage. And a change like this would probably fail in a large way if it were to cause a problem.

@benmccann
Copy link
Contributor Author

Given that we ran it through a bunch of testing and that it'd be nice to release early and often, am I able to convince you to cut a release now? :-) If you have any other concerns I'll do my best to address them

@benmccann benmccann deleted the mvn3 branch July 27, 2014 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
improvement
Projects
None yet
Milestone
1.5
Development

Successfully merging this pull request may close these issues.
5 participants
@benmccann @tcurdt @tmortagne @muuki88 @ebourg