triton-inference-server / server

The Triton Inference Server provides an optimized cloud and edge inferencing solution.
https://docs.nvidia.com/deeplearning/triton-inference-server/user-guide/docs/index.html
BSD 3-Clause "New" or "Revised" License
8.39k stars 1.49k forks source link

feat: Add copyright hook #7666

Closed pranavm-nvidia closed 1 month ago

pranavm-nvidia commented 2 months ago

What does the PR do?

Adds a copyright pre-commit hook (see below sections for details)

Checklist

Commit Type:

Check the conventional commit type box here and add the label to the github PR.

Where should the reviewer start?

The main files to be reviewed here are tools/add_copyright.py, which provides a new pre-commit hook that will update or add copyright headers to new or modified files, and .pre-commit-config.yaml. The logic is more complicated than one might expect because the repository includes many different types of files that require slightly different handling.

Caveats:

I've only added support for the most common file types that I saw in the repo. Note that it is usually trivially simple to add support for new file types using the building blocks I've provided in the hook implementation.

Background

Right now, there is no automatic way of updating/inserting copyright headers in the various source files in the repository. Consequently, some files have incorrect copyright years and many don't include headers at all. With this hook, the header no longer needs to be added or changed manually and will automatically be updated when the file is touched.

pranavm-nvidia commented 1 month ago

@rmccorm4 I can revert the changes to the copyright headers if it makes it easier to review. Let me know what the rules are for copyrights and I can implement those. My understanding right now is that it should be of the format Copyright <optional_first_modified_year>-<last_modified_year>

rmccorm4 commented 1 month ago

I believe you will find the following formats throughout the code:

  1. Copyright (c) YYYY - (YYYY - creation year == last modified year) never touched in a new year after creation year
  2. Copyright YYYY - same as (1), but I don't know if these should be considered invalid or not yet. Ideally we would standardize to one or the other for simplicity - but need legal confirmation most likely.
  3. Copyright YYYY-ZZZZ - (YYYY - creation year), (ZZZZ - last modified year), Intentionally removed (c) to avoid line wrapping with multi-year

I think supporting all of these (if possible) would provide the least number of files changed unnecessarily for review with max utility - and then I can separately follow-up to see if we standardize on (1) or (2) after confirmation with legal and not allow both.

rmccorm4 commented 1 month ago

CC @GuanLuo for viz

pranavm-nvidia commented 1 month ago

@rmccorm4 I think this is ready for review now. See the included tests for what the expected behavior is with the various copyright formats.

oandreeva-nv commented 1 month ago

lgtm in general, left some questions/suggestions

rmccorm4 commented 1 month ago

@oandreeva-nv good to ship it?