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

Add copyright/license headers to all scripts #5827

Closed
yurishkuro opened this issue Aug 11, 2024 · 2 comments · Fixed by #5829
Closed

Add copyright/license headers to all scripts #5827

yurishkuro opened this issue Aug 11, 2024 · 2 comments · Fixed by #5829
Labels
changelog:new-feature Change that should be called out as new feature in CHANGELOG enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement

Comments

@yurishkuro
Copy link
Member

yurishkuro commented Aug 11, 2024

In our make lint and make fmt targets we ensure that all Go source files have a copyright & license comments like this:

$ head -2 cmd/jaeger/main.go
// Copyright (c) 2023 The Jaeger Authors.
// SPDX-License-Identifier: Apache-2.0

Objective:

  • modify ./scripts/updateLicense.py to also work for shell scripts under ./scripts/
  • make sure not to override the shebang line
  • also make sure this script works for the Makefiles, which have the pattern of Makefiles* or *.mk
  • invoke the script as needed from lint and fmt targets

Verification:

  • running make lint should flag all the scripts and Makefiles that are missing the license headers
  • running make lint should update them as needed

DO NOT commit the changes to the file headers, only raise a PR containing changes to the script & make targets.

@yurishkuro yurishkuro added help wanted Features that maintainers are willing to accept but do not have cycles to implement good first issue Good for beginners labels Aug 11, 2024
@dosubot dosubot bot added changelog:new-feature Change that should be called out as new feature in CHANGELOG enhancement labels Aug 11, 2024
@Zen-cronic
Copy link
Contributor

Zen-cronic commented Aug 12, 2024

i believe the ALL_SRC variable in Makefile needs to be updated too? So that the .sh files are auto reformatted.
Something like:

ALL_SRC = $(shell find . \( -name '*.go' -o -name '*.sh' \) 
                   -not -name '_*' \
                   -not -name '.*' \
                    ... // the rest

Also, are the .py scripts included?

@yurishkuro
Copy link
Member Author

Yes py scripts are included

JaredTan95 pushed a commit to JaredTan95/jaeger that referenced this issue Aug 13, 2024
## Which problem is this PR solving?
- Resolves jaegertracing#5827

## Description of the changes
- Update the `update_go_license` function to handle the `.sh` files and
makefiles
- Update the
[ALL_SRC](https://github.com/jaegertracing/jaeger/blob/dc49b1faaeee15c2498c562309bab4fc0525bd56/Makefile#L32)
variable in the main `Makefile` to include the new file types, which in
turn updates the `lint` and `fmt` targets

## How was this change tested?
-

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Jared Tan <jian.tan@daocloud.io>
JaredTan95 pushed a commit to JaredTan95/jaeger that referenced this issue Aug 14, 2024
## Which problem is this PR solving?
- Resolves jaegertracing#5827

## Description of the changes
- Update the `update_go_license` function to handle the `.sh` files and
makefiles
- Update the
[ALL_SRC](https://github.com/jaegertracing/jaeger/blob/dc49b1faaeee15c2498c562309bab4fc0525bd56/Makefile#L32)
variable in the main `Makefile` to include the new file types, which in
turn updates the `lint` and `fmt` targets

## How was this change tested?
-

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Jared Tan <jian.tan@daocloud.io>
JaredTan95 pushed a commit to JaredTan95/jaeger that referenced this issue Aug 28, 2024
## Which problem is this PR solving?
- Resolves jaegertracing#5827

## Description of the changes
- Update the `update_go_license` function to handle the `.sh` files and
makefiles
- Update the
[ALL_SRC](https://github.com/jaegertracing/jaeger/blob/dc49b1faaeee15c2498c562309bab4fc0525bd56/Makefile#L32)
variable in the main `Makefile` to include the new file types, which in
turn updates the `lint` and `fmt` targets

## How was this change tested?
-

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

---------

Signed-off-by: Kaung Zin Hein <kaungzinhein113@gmail.com>
Signed-off-by: Yuri Shkuro <github@ysh.us>
Co-authored-by: Yuri Shkuro <github@ysh.us>
Signed-off-by: Jared Tan <jian.tan@daocloud.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
changelog:new-feature Change that should be called out as new feature in CHANGELOG enhancement good first issue Good for beginners help wanted Features that maintainers are willing to accept but do not have cycles to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants