-
-
Notifications
You must be signed in to change notification settings - Fork 931
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
SAK-40128 Merge SU admin tools into one project. #5688
Conversation
axxter99
commented
Jun 13, 2018
- ./tool/tool-tool/su => ./admin-su
- pom.xlm: artifactI:admin-tool-su
1) ./tool/tool-tool/su => ./admin-su 2) pom.xlm: artifactI:admin-tool-su
@buckett ? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Much better to make this code more discoverable.
pom.xml
Outdated
@@ -111,7 +111,7 @@ | |||
<module>syllabus</module> | |||
<module>taggable</module> | |||
<module>textarea</module> | |||
<module>tool</module> | |||
<module>admin-su</module> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move this line to keep them alphabetical?
pom.xml
Outdated
@@ -194,7 +194,7 @@ | |||
<module>syllabus</module> | |||
<module>taggable</module> | |||
<module>textarea</module> | |||
<module>tool</module> | |||
<module>admin-su</module> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we move this line to keep them alphabetical?
admin-su/pom.xml
Outdated
<groupId>org.sakaiproject</groupId> | ||
<artifactId>sakai-tool-tool-su</artifactId> | ||
<groupId>org.sakaiproject.su</groupId> | ||
<artifactId>admin-tool-su</artifactId> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's generally better if the folder name matches the artifactId
, I know Sakai is currently bad at this but would be nice if we could start the path to making it better.
https://blog.sonatype.com/2011/01/maven-tip-project-directories-and-artifact-ids/
Also currently has conflicts. |
@buckett, are you ok with the changes here? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, looks good.
* SAK-40128 Merge SU admin tools into one project. 1) ./tool/tool-tool/su => ./admin-su 2) pom.xml: artifactId:admin-tool-su
* SAK-40128 Merge SU admin tools into one project. 1) ./tool/tool-tool/su => ./admin-su 2) pom.xml: artifactId:admin-tool-su
* SAK-40128 Merge SU admin tools into one project. 1) ./tool/tool-tool/su => ./admin-su 2) pom.xml: artifactId:admin-tool-su