Skip to content

Google issues: - #1

Closed
mathieucarbou wants to merge 4 commits into
tadglines:masterfrom
Ovea:master
Closed

mathieucarbou wants to merge 4 commits into
tadglines:masterfrom
Ovea:master

Conversation

@mathieucarbou

Copy link
Copy Markdown
Contributor

@tadglines

Copy link
Copy Markdown
Owner

The code changes seem ok, I'll take a closer look tomorrow. There are two problems. One, some of the file relocations didn't get marked as moves, and are instead showing and delete + add. And two, ALL copyright notices have been modified. I have no problem with you claiming copyright to files you created, but I'm not happy with you claiming copyright on files you didn't author.

@mathieucarbou

Copy link
Copy Markdown
Contributor Author

Hi,
For the copyright I completely agree with you: this is because the Maven License Plugin is used, which check licenses before deploying on e Maven repository. It has not been configured correctly and since we had to deploy a snapshot for our project I've put the config so that newly created files are OK, but for old files I should put exclusions.

For the parent pom you'll also notice that the distribution repository and company has to be changed. Since our fork is deployed into an Ovea repository, this is our "own" config that must be changed.

For the add + delete, this is because the Maven structure was already ready in a git repository (which was not a fork) before you imported in github. So to send you pull request, I had to refork and move the new structure with the fork and delete the old structure.

This is the first and last time this will happen because now the repo are correctly setup. What I would suggest for the merge is that you checkout our fork in an other folder and move the file in you github acordingly to our structure in our fork. Also, you can comment the maven license plugin.

Thus you'll resolve all these issues at one time. I'll take care to improve the MLP configuration to add correct exclusions.

Thanks,

Mathieu.

@tadglines

Copy link
Copy Markdown
Owner

I've pushed most of the changes presented in this pull request.
Regarding the conversion to maven, I did make a couple of changes. I remove the testatoo thing and added explicit jetty dependencies and the jetty-mavin-plugin. Also, I move you and David into the contributors section of the root POM. I suspect that at some point in the future your contributions will be significant enough that I'll make you a co-owner of the project, but for now I prefer to remain the only committer. This is only the second open-source project I've been involved in and only the first I created. So bear with me if I seem less cooperative than you want.

Regarding the last change in this pull request, I forked the Socket.IO project and all my changes are mastered there. The socket.io.js file in the Socket.IO-Java project is just a copy of the latest "compiled" version from http://github.com/tadglines/Socket.IO. Please create a push request to that project for the WEB_SOCKET_WSF_LOCATION change and I'll push it there then into this project.

@mathieucarbou

Copy link
Copy Markdown
Contributor Author

Hi.
Thank you !
I'll synchronize to those changes.

This pull request was closed.
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.

2 participants