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

Add a build file for the main code #15

Merged
merged 1 commit into from
Aug 1, 2014

Conversation

benmccann
Copy link
Contributor

Add a build file for the main code. Make the sample project depend on this jar so that we don't need to update two copies of the code when making changes. Update the README to describe this new setup and convert it to a markdown file for better formatting

@pitbulk
Copy link
Contributor

pitbulk commented Aug 1, 2014

In sample/pom.xml , the in the line 22,

<finalName>java-saml</finalName>

must be renamed by

<finalName>java-saml-sample</finalName>

?

@pitbulk
Copy link
Contributor

pitbulk commented Aug 1, 2014

I created this branch https://github.com/onelogin/java-saml/tree/restructure

We want to maintain the com folder at the top of the Toolkit and we will advise that this com/ must be moved inside sample/src/main/java/

@benmccann
Copy link
Contributor Author

@pitbulk okay. i'm not sure why you want to have the src in com instead of the more common src/main/java/com, but I have made that change

The reason I renamed the project from java-saml to java-saml-sample is that the new pom file uses the name java-saml and you can't have two projects with the same name and group id. I thought it made much more sense for the main src code to use this name and for the sample web app to be named java-saml-sample

@pitbulk
Copy link
Contributor

pitbulk commented Aug 1, 2014

Ok, I'm agree with the src/main/java/com. Can you review the branch:
https://github.com/onelogin/java-saml/tree/restructure

Is as expected? Can you review also the line:
https://github.com/onelogin/java-saml/blob/restructure/sample/pom.xml#L28

is

<finalName>java-saml-sample</finalName>

ok? or must be renamed to

<finalName>java-saml</finalName>

?

My idea if is you are agree I will replace the master branch by restructure branch.

… this jar so that we don't need to update two copies of the code when making changes. Update the README to describe this new setup and convert it to a markdown file for better formatting
@benmccann
Copy link
Contributor Author

Ok, I updated this pull request and rebased it from master. You may find it easier to merge this than the other branch because it has a couple changes like spacing in the pom.xml and a couple sentences in the README are a bit different per my comments on the other branch. Either way should be fine though as they both seem to be good improvements.

pitbulk added a commit that referenced this pull request Aug 1, 2014
Restructure the java-toolkit
@pitbulk pitbulk merged commit 0513e27 into SAML-Toolkits:master Aug 1, 2014
@pitbulk
Copy link
Contributor

pitbulk commented Aug 1, 2014

Ok, I merged your branch, can you check it and fix the Attr problem?

@benmccann
Copy link
Contributor Author

Thanks! It looks good. I just sent a PR for the Attr problem and a minor update to the README

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