Automating Addition of License Headers #88
Conversation
Accumulo Proxy is now able to build when depending on Accumulo 2.1.3
Update to newest accumulo
Kidcredible300
force-pushed
the
LicenceHeaders
branch
from
ebb76d1 to
ad1bc77
Compare
|
I'm not sure how to remove the commits that show me merging changes from main. |
|
My Pull Request is not acting as expected, I'm going to mark this as a draft for now. |
keith-turner
left a comment
keith-turner
left a comment
There was a problem hiding this comment.
These changes look good, only thing I am uncertain about is if we need to explicitly specify the license text like the accumulo pom does.
| # specific language governing permissions and limitations | ||
| # under the License. | ||
| # | ||
| # Autogenerated by Thrift Compiler (0.17.0) |
There was a problem hiding this comment.
This comment generated by thrift is being removed by the plugin, which is probably fine. I looked at the docs for the plugin and looked at how accumulo is using the plugin and did not see any way to avoid removing this.
There was a problem hiding this comment.
Did see the following that may provide a way to solve this, but not sure if anything workable could be created using that.
https://mathieu.carbou.me/license-maven-plugin/#changing-header-style-definitions
Not something that has to be done for this PR, just commenting on something I saw.
There was a problem hiding this comment.
I fixed this in the commit I added in 5461a39. It was caused by the generate-thrift.sh script, which was mangling everything around the shebang lines for the python and ruby files.
| </plugins> | ||
| </pluginManagement> | ||
| <plugins> | ||
| <plugin> |
There was a problem hiding this comment.
The accumulo pom specifies license text here. This seems to somehow know to use the apache license w/o doing that, maybe its using the license section earlier in the pom?
There was a problem hiding this comment.
It looks like this repo has a copy of the license header in the contrib directory, and had configuration already set up to do this... it just didn't have an execution for it. https://github.com/apache/accumulo/blob/3a0d8e5c72915fb4c7ce26d6156d15b0a444fb71/pom.xml#L669
* Manually update a few files that the plugin ignores (like .github/, .git*, and markdown files) * Update copyright date (unrelated, but good to do, so it's not lost) * Update the license plugin and modernize the config * Add some mappings for special files (Gemfile, Dockerfile, etc.) * Change default mapping for cpp files to use single star * Update rat and license execution order by setting their phases to run after thrift code generation * Use https in a few xml schema locations (unrelated, but good to do) * Remove the license header generation in generate-thrift.sh; this leaves the responsibility of license headers to the license plugin and addresses quirks, like mangling of the shebang lines and conflict with the Autogenerated comments, as well as the appearance of duplicate license headers before/after shebangs * Remove duplicate license headers in .thrift files * Remove Eclipse template file (no longer used by the project; maven plugins are used for formatting/license headers)
|
Thanks for this PR, @Kidcredible300! I built on what you did, but added commit 5461a39 that fixed some additional issues. I didn't see you listed on our "people" page, acknowledging contributors. If you wish to be added as a contributor to https://accumulo.apache.org/people/ , please open a pull request to add yourself at https://github.com/apache/accumulo-website/edit/main/pages/people.md and leave a reference to If you intend to be a regular contributor to Accumulo projects, please consider subscribing to our developer mailing list (https://accumulo.apache.org/contact-us/) and introducing yourself. 😺 |
3 participants
Using the com.mycila Maven Plugin, automated the addition of Apache License Headers to files in the project. This is the same tool as the one used in the main Accumulo project. License headers are added during the "process-sources" phase.
Meant to resolve issue #56