Skip to content

Conversation

sagenschneider
Copy link
Contributor

No description provided.

@blee-techempower
Copy link
Contributor

Passed local test.

 Test: officefloor
|       fortune     : PASS
|       plaintext   : PASS
|       db          : PASS
|       update      : PASS
|       json        : PASS
|       query       : PASS

blee-techempower added a commit that referenced this pull request Nov 3, 2015
OfficeFloor implementation of Framework Benchmark Performance Tests.
@blee-techempower blee-techempower merged commit 0148c88 into TechEmpower:master Nov 3, 2015
@msmith-techempower
Copy link
Member

@sagenschneider I had to revert your pull request as it did not conform to our standards. I will leave comments on the files needing changes here, but you will have to open a new pull request to get OfficeFloor added.

Copy link
Member

Choose a reason for hiding this comment

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

Please delete everything about this and replace with:

fw_depends java mvn

We are presently in the midst of opening a pull request to fix up the various java version problems (likely, the culprit behind why you did your dependencies this way), so once we get that in (hopefully, today or tomorrow) you can open your new pull request.

@sagenschneider
Copy link
Contributor Author

No worries. Please excuse this if it has caused too many problems. I'm happy to make the change once the issues are resolved. Please either update this PR once done, or email me at daniel@officefloor.net and I'll resubmit with changes.

Also, can I confirm that the fix will have:

  • "fw_depends java8" use the java8 libraries. Current implementation seemed to replace the libraries with Java 7 versions.
  • "fw_depends mvn" will pull in maven 3.1.1 (or possibly higher). There is a bug in mvn 3.0.5 regarding depending on project source dependences causing the OfficeFloor plugin to not start the test application (3.0.5 tries to unzip the source directory).

Note: the above is fixes if that helps.

@msmith-techempower
Copy link
Member

Really, the only problem was the manual installation of java and maven; we provide fw_depends to ensure the environment is built in a way that is easily cleaned.

Additionally, the java8-vs-java7 issues should be resolved by a pull request soon, and I will ping you here when you can rebase and open another pull request.

@sagenschneider
Copy link
Contributor Author

Would you accept a pull request change to toolset/setup/linux/systools/maven.sh setup that is following? (this will load latest maven)

#!/bin/bash

RETCODE=$(fw_exists ${IROOT}/maven.installed)
[ ! "$RETCODE" == 0 ] || {
source $IROOT/maven.installed
return 0; }

sudo add-apt-repository "deb http://ppa.launchpad.net/natecarlson/maven3/ubuntu precise main"
sudo apt-get update
sudo apt-get -y --force-yes install maven3
if [ -e /usr/bin/mvn ]
then
sudo rm -f /usr/bin/mvn
fi
sudo ln -s /usr/share/maven3/bin/mvn /usr/bin/mvn

mvn -version

touch $IROOT/maven.installed

source $IROOT/maven.installed

@msmith-techempower
Copy link
Member

Why? Does OfficeFloor require a particular version or maven?

@sagenschneider
Copy link
Contributor Author

I am running OfficeFloor through the OfficeFloor maven plugins. The OfficeFloor plugins load all necessary dependencies, however due to a bug in 3.0.5 the maven dependency loading fails on the source directory when pulling in (as assumes everything to be jars). This has been resolved in later maven version. Note: maven 3.0.5 is nearly 3 years old.

@sagenschneider
Copy link
Contributor Author

PR #1911 has the request changes for OfficeFloor to use "fw_depends java mvn".
Also, PR #1912 has the changes for using the latest version of maven.

@sagenschneider sagenschneider deleted the techempire_master branch August 17, 2018 05:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants