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.

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

Issue 7 #9

Merged
merged 25 commits into from
Jan 24, 2017
Merged

Issue 7 #9

merged 25 commits into from
Jan 24, 2017

Conversation

cplerch
Copy link
Contributor

@cplerch cplerch commented Jan 23, 2017

Please review and merge into (new) branch "develop". Then test thoroughly on Linux, InelliJ and standalone Gradle wrapper.

@belgattitude
Copy link
Owner

Hi @cplerch,

Unbelievable ! Looks you did much more than anticipated and reviewing after change in the directory structure will take some time.

So my idea is to basically review the P/R and merge it on Tuesday. I'm quite confident in the way you work... Of course If I find something, I'll give you feedback, just cannot match your knowlegde on this. I'll try my best ;)

After the merge, I'll try to update the doc and changelog. I really care for that part. I'll need you to review this. But let's talk soon and thanks a lot.

@cplerch
Copy link
Contributor Author

cplerch commented Jan 23, 2017 via email

@cplerch
Copy link
Contributor Author

cplerch commented Jan 24, 2017 via email

@belgattitude belgattitude merged commit 312e8ed into belgattitude:develop Jan 24, 2017
@belgattitude
Copy link
Owner

belgattitude commented Jan 24, 2017

Hi @cplerch ,

Just merged.

I'm not totally sure about changing the namespace in io.soluble. I hoped to get contact with at least one original maintainer (haven't had reply yet and don't think it will happen)... So yes why not, I see some advantages at least (like distinguish the issues on stackoverflow, for example...).

For the little story, I own the soluble.io domain that I'll keep for opensource initiatives or projects following some standards. Must confess that I was inspired by what they did with https://thephpleague.com/. At some point there will be a website with some of my (or others) work sharing the soluble namespace.

So ok let's go for it, I suppose If you did is that you feel the need to clearly differenciate the work. Just wanted to share my hesitations.

So in the following hours, I'll review, update doc, prepare changelog, credits... and give you feedback.

@belgattitude
Copy link
Owner

A note if you intend to work on the war...

I guess you realized, the build includes the production of both JavaBridgeTemplate.war (500k) and JavaBridge.war (47Mb). The latter including various examples and tests with very outdated libs (Eclipse Birt, iText)... All of those resources being present in the './unsupported' directory (big one).

For my part, this build makes non-sense. My naive plan is to move examples of functionalities somewhere else (or dedicate a starter repo, like the pjb-starter-gradle). Remove the build and './unsupported' at some point (we'll have to be careful of the JavaDoc build, maybe some links somewhere).

There is no real need now, but we should discuss it and surely not invest time to make it work.

@belgattitude
Copy link
Owner

belgattitude commented Jan 24, 2017

Hi @cplerch,

Seems the build is broken on my side due to missing Java.inc... I guess you have it in your src/main/resources/META-INF/java/Java.inc folder. But it's not built with the latests changes.

As debug notes with ant (I see more clearly than with gradle, but it's the same)

ant genJavaInc -Dphp_exec=php5.6 -v
# Warning: Could not find file .../src/main/resources/META-INF/java/Java.inc to copy

I also pushed an updated CHANGELOG.md where we can add our changes. Good to use it to keep in sync.

@cplerch
Copy link
Contributor Author

cplerch commented Jan 24, 2017 via email

@cplerch
Copy link
Contributor Author

cplerch commented Jan 24, 2017 via email

@belgattitude
Copy link
Owner

Great, whenever ready submit a P/R or why not work on the develop branch directly ?, you should have access. Also note that after a build, those files are created but not sure what to do with them

image

Any thought ?

@cplerch
Copy link
Contributor Author

cplerch commented Jan 24, 2017 via email

@belgattitude belgattitude mentioned this pull request Feb 3, 2017
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.

None yet

2 participants