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

License headers fixed or added using mvn license:format #8

Merged
merged 7 commits into from Jun 11, 2015

Conversation

ppalaga
Copy link
Contributor

@ppalaga ppalaga commented Jun 10, 2015

No description provided.

@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 10, 2015

Ah, it is failing because Travis relies in maven central only. Let me fix it.

@@ -7,7 +7,16 @@ local.properties
.idea/
*.iml

# Eclipse
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure we should keep it for Studio-based project, but if it is more comfortable for you — not a problem.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not sure what Studio-based should imply here... Is this not a usual Gradle project that can be handled by any Gradle-compatible tool incl. Eclipse?

Copy link
Contributor

Choose a reason for hiding this comment

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

As far as I know Eclipse doesn’t support it — at least yet. You can keep, not a big deal.

@arturdryomov
Copy link
Contributor

These directories and files should be ignored, because they are auto-generated.

  • gradle
  • gradlew
  • gradlew.bat
  • *.properties — not in the repository, but are generated after building a project.

@arturdryomov
Copy link
Contributor

@ppalaga Should I fix checkstyle warnings in your branch somehow or just merge and fix afterwards?

@arturdryomov
Copy link
Contributor

Another two nits.

  • Should the licence header be followed by an empty line like there?
  • Should I use mvn -s .travis.maven.settings.xml verify locally as well?

@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 10, 2015

These directories and files should be ignored, because they are auto-generated.
gradle gradlew gradlew.bat *.properties — not in the repository, but are generated after building a project.

Hm... some of them were in git even before my changes. Let me try to remove them.

Should the licence header be followed by an empty line?

If mvn license:format did without an empty line, I'd say that's correct.

Should I use mvn -s .travis.maven.settings.xml verify locally as well?

Yes, you can, but when thinking of it again, it downloads a lot of maven stuff that is not necessary for the license plugin and checkstyle. Let me try to find a more sparse form of the mvn command.

@arturdryomov
Copy link
Contributor

Hm... some of them were in git even before my changes. Let me try to remove them.

I meant ignored by checking plugins.

@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 10, 2015

Hm... some of them were in git even before my changes. Let me try to remove them.

I meant ignored by checking plugins.

OK, I excluded them from license checks ppalaga@edac4f7 I hope the list is complete

@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 10, 2015

The last commit is a result of bulk Source > Format operation in Eclipse. Not sure if this will suit Android Studio. Feel free to criticize or propose something else. Anyway, Checkstyle is passing like that. 13c52e0

@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 10, 2015

Should I use mvn -s .travis.maven.settings.xml verify locally as well?

I changed to mvn license:check checkstyle:check However, I am not 100% sure it saved any bandwith.

@arturdryomov
Copy link
Contributor

marvelous

@ppalaga Is the .travis.maven.settings.xml really necessary though? Seems like it just adds repositories and the hawkular-parent is available at Central. Isn’t it better to move these declarations to the parent POM directly?

@ppalaga
Copy link
Contributor Author

ppalaga commented Jun 11, 2015

@ming13 yes, .travis.maven.settings.xml makes sense, because hawkular-parent reaches maven central within 24hrs from the release to JBoss Nexus and I typically upgrade parent in consuming projects just after releasing to nexus.

@arturdryomov
Copy link
Contributor

@ppalaga, thanks for your work!

arturdryomov pushed a commit that referenced this pull request Jun 11, 2015
License headers fixed or added using mvn license:format
@arturdryomov arturdryomov merged commit ca10081 into hawkular:master Jun 11, 2015
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