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

standardize license header #77

Merged
merged 6 commits into from
Mar 13, 2024
Merged

standardize license header #77

merged 6 commits into from
Mar 13, 2024

Conversation

spolti
Copy link
Contributor

@spolti spolti commented Dec 4, 2023

chore: Format all *.go files to have the empty line above the package declaration
The goal is to add a new step to take care of the license headers as part
of the go fmt make goal to keep it consistent across all files it is present.

Motivation

Modifications

Result

@oss-prow-bot oss-prow-bot bot requested review from njhill and rafvasq December 4, 2023 21:04
@spolti spolti marked this pull request as draft December 4, 2023 21:04
@spolti spolti requested a review from ckadner December 5, 2023 13:57
@spolti spolti marked this pull request as ready for review December 5, 2023 13:57
@oss-prow-bot oss-prow-bot bot requested a review from tjohnson31415 December 5, 2023 13:57
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Nice! Nifty script. Thanks @spolti

I still need to do a proper test, just a few comments for now

scripts/addheaders.sh Outdated Show resolved Hide resolved
scripts/addheaders.sh Outdated Show resolved Hide resolved
scripts/addheaders.sh Outdated Show resolved Hide resolved
scripts/addheaders.sh Outdated Show resolved Hide resolved
@spolti spolti requested a review from ckadner December 14, 2023 18:29
@spolti
Copy link
Contributor Author

spolti commented Jan 26, 2024

Hi @ckadner did you had time to test it?

scripts/addheaders.sh Outdated Show resolved Hide resolved
scripts/addheaders.sh Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
chore:	Format all *.go files to have the empty line above the package declaration
	The goal is to add a new step to take care of the license headers as part
	of the `go fmt` make goal to keep it consistent across all files it is present.

Signed-off-by: Spolti <[email protected]>
Signed-off-by: Spolti <[email protected]>
@spolti
Copy link
Contributor Author

spolti commented Feb 29, 2024

Thanks for the review @ckadner, I've applied your suggestions, please take a look.

Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

So close! -- Apologies once again for my tardy review :-(

If you want I can just add the missing script line and merge?

scripts/add_headers.sh Outdated Show resolved Hide resolved
scripts/add_headers.sh Outdated Show resolved Hide resolved
@spolti spolti requested a review from ckadner March 12, 2024 18:24
@spolti spolti force-pushed the issue74 branch 2 times, most recently from c6c3dbd to ce9ef7c Compare March 12, 2024 19:08
Signed-off-by: Spolti <[email protected]>
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Thanks @spolti -- works, but not sure we might want to cleanup

scripts/add_headers.sh Outdated Show resolved Hide resolved
scripts/add_headers.sh Outdated Show resolved Hide resolved
@spolti
Copy link
Contributor Author

spolti commented Mar 13, 2024

Thanks @spolti -- works, but not sure we might want to cleanup

what do you mean by the cleanup?

@spolti spolti requested a review from ckadner March 13, 2024 18:10
Copy link
Member

@ckadner ckadner left a comment

Choose a reason for hiding this comment

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

Great! Thanks @spolti

Copy link

oss-prow-bot bot commented Mar 13, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ckadner, spolti

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@ckadner ckadner merged commit 25fe3bd into kserve:main Mar 13, 2024
6 of 7 checks passed
@spolti spolti deleted the issue74 branch March 13, 2024 19:31
@spolti
Copy link
Contributor Author

spolti commented Mar 13, 2024

Thanks for all the effort you put on reviewing this @ckadner.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format all *.go files to have the empty line above the package declaration
2 participants