apache / incubator-hugegraph

A graph database that supports more than 100+ billion data, high performance and scalability (Include OLTP Engine & REST-API & Backends)
https://hugegraph.apache.org
Apache License 2.0
2.58k stars 512 forks source link

fix(server): unify the license headers #2438

Closed msgui closed 5 months ago

msgui commented 5 months ago

Purpose of the PR

Main Changes

Verifying these changes

Does this PR potentially affect the following parts?

Documentation Status

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (1d4532c) 66.23% compared to head (9ed3749) 61.39%. Report is 1 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2438 +/- ## ============================================ - Coverage 66.23% 61.39% -4.84% + Complexity 828 488 -340 ============================================ Files 511 511 Lines 42597 42598 +1 Branches 5942 5942 ============================================ - Hits 28215 26154 -2061 - Misses 11566 13759 +2193 + Partials 2816 2685 -131 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

msgui commented 5 months ago

fine, wonder to know all the files header are updated?

This PR solely updates the license header in hg-server/hg-api.

imbajin commented 5 months ago

fine, wonder to know all the files header are updated?

This PR solely updates the license header in hg-server/hg-api.

Get it, license header could update together (due to it is almost a completely repetitive task, just take a few samples for inspection and confirmation, no need to check them one by one)

VGalaxies commented 5 months ago

@msgui I noticed that some configuration files, such as hugegraph-server/hugegraph-test/src/main/resources/hugegraph.properties, do not have corresponding modified license headers. Is this expected?

msgui commented 5 months ago

@msgui I noticed that some configuration files, such as hugegraph-server/hugegraph-test/src/main/resources/hugegraph.properties, do not have corresponding modified license headers. Is this expected?

thanks for the reminder

msgui commented 5 months ago

this commit includes files of all formats for easy review. image 👉:47e2fe24976efe092fd6f944444be00ba142cba5

VGalaxies commented 5 months ago

Also, these extra blank lines here can be removed, it seems? image (compared to https://github.com/apache/incubator-hugegraph/wiki/HugeGraph-Code-Style-Guide)

msgui commented 5 months ago

PTAL~

  1. It seems reasonable to consider updating the license header for the entire repository, not just under hugegraph-server.
  2. For the license headers only under hugegraph-server, there still seem to be some missed files. I have conducted a preliminary count (just a reminder): image (expected: 791, actual: 783)

Thank you for the reminder. The license header format in the remaining files is already correct, so no further corrections are needed

eg: hugegraph-server/Dockerfile

VGalaxies commented 5 months ago

@imbajin could also check again~

Additionally, I believe that performing the license header check in the current CI (licence-checker.yml) seems somewhat delayed. It might be worth considering moving it to the verify stage. This way, developers can discover related errors earlier during local builds.

imbajin commented 5 months ago

@imbajin could also check again~

Additionally, I believe that performing the license header check in the current CI (licence-checker.yml) seems somewhat delayed. It might be worth considering moving it to the verify stage. This way, developers can discover related errors earlier during local builds.

Good suggestion, could mark it as a TODO (enhance our CI to check/validate style problems)

liuxiaocs7 commented 5 months ago

This tool hawkeye may help license header check and format?

VGalaxies commented 4 months ago
  • [x] keep the dist/*/bin/*.properties (user config files) clean (no need to add header here)

@imbajin Is it only for cleaning the license header in properties files in user configuration files here? I found that some other user configuration files have license headers.

image

Also, do properties files in the conf-raft* directory need to have their license headers cleaned?

image

imbajin commented 4 months ago
  • [x] keep the dist/*/bin/*.properties (user config files) clean (no need to add header here)

@imbajin Is it only for cleaning the license header in properties files in user configuration files here? I found that some other user configuration files have license headers.

image

@VGalaxies The current judgment points are:

  1. If a configuration file is not frequently used/modified by users, following ASF's suggestion may be better to add a file header to reduce disputes
  2. Otherwise(In HG), considering that there are already many configuration files and the content is relatively long, the commonly used configurations by users are not separately added to avoid affecting the user experience
  3. (like point 1) Configuration files from testing/and other modules could also add headers (although not required)

Also, do properties files in the conf-raft* directory need to have their license headers cleaned?

image

So these places seem to fit the 1st & 3rd points and can preserve license headers.