Fix lint errors by Warashi · Pull Request #5897 · pipe-cd/pipecd

Warashi · 2025-05-29T05:45:11Z

What this PR does:

fixed lint error about import groupings
fixed lint error about log.Fatal and defer
- log.Fatal calls os.Exit, so the deferred functions are not called. instead of using log.Fatal, I use the log.Println, made func _main() int and called os.Exit(_main()) in the main.
- for detail, please see the following commits
fixed lint error about simplicity

Why we need it:

To make lint success on local
To hardening codebase

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?: No

How are users affected by this change:
Is this breaking change:
How to migrate (if breaking change):

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

log.Fatalf calls os.Exit(1) and deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

log.Fatal calls os.Exit, so the deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. This is a workaround to fix the lint errors. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

codecov · 2025-05-29T05:46:30Z

Codecov Report

Attention: Patch coverage is 1.42857% with 69 lines in your changes missing coverage. Please review.

Project coverage is 27.88%. Comparing base (23b62f8) to head (d51b7bd).
Report is 17 commits behind head on master.

Files with missing lines	Patch %	Lines
tool/actions-gh-release/main.go	0.00%	31 Missing ⚠️
tool/actions-plan-preview/main.go	0.00%	30 Missing ⚠️
tool/actions-gh-release/filematcher.go	0.00%	6 Missing ⚠️
tool/actions-gh-release/release.go	50.00%	1 Missing ⚠️
tool/codegen/protoc-gen-auth/main.go	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #5897      +/-   ##
==========================================
- Coverage   27.89%   27.88%   -0.02%     
==========================================
  Files         519      519              
  Lines       55900    55923      +23     
==========================================
  Hits        15594    15594              
- Misses      39108    39131      +23     
  Partials     1198     1198

Flag	Coverage Δ
.	`23.73% <ø> (ø)`
.-pkg-app-pipedv1-plugin-example	`0.00% <ø> (ø)`
.-pkg-app-pipedv1-plugin-kubernetes	`63.32% <ø> (ø)`
.-pkg-app-pipedv1-plugin-kubernetes_multicluster	`64.93% <ø> (ø)`
.-pkg-app-pipedv1-plugin-wait	`35.51% <ø> (ø)`
.-pkg-plugin-sdk	`49.25% <ø> (ø)`
.-tool-actions-gh-release	`19.23% <2.56%> (-0.36%)`	⬇️
.-tool-actions-plan-preview	`22.83% <0.00%> (-0.40%)`	⬇️
.-tool-codegen-protoc-gen-auth	`0.00% <0.00%> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

khanhtc1202 · 2025-05-29T05:57:55Z


 	for _, ctg := range cfg.CommitCategories {
-		commits := make([]ReleaseCommit, 0, 0)
+		commits := make([]ReleaseCommit, 0)


Suggested change

commits := make([]ReleaseCommit, 0)

commits := make([]ReleaseCommit, 0, len(filteredCommits))

nits

Thank you. Fixed on this commit.
2465b2e

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

khanhtc1202 · 2025-05-29T06:05:22Z

 		if err != nil {
-			log.Fatal(err)
+			log.Println(err)
+			return err


IMO, if this error is returned, and the caller prints again, it's redundant. If we don't have specific logic based on the error, then instead of returning the error, we should return the existing code. WDYT?

I'm unsure about that.

Initially, I wrote as you said and thought it was not the standard go code to check whether the return value was zero.
Secondly, I wrote just return the error from the doComment, printed it on the caller side, and was a bit nervous about whether I had written all the logging.
Finally, I wrote logging in the doComment, returned an error, and didn't print it on the caller side.

I see. Thanks for the clarification. Because I saw many places that perform "if error, then return 1, else return 0," I think it could be better to emit it as well. This means we logged the error and returned 0 and 1 from this doComment, and on the caller side, only returning doComment is enough.

OK, I try to do it again.

I changed doComment to return int. I think it's simpler than before. Thank you.
7f14722

t-kikuc

Almost LGTM

t-kikuc · 2025-05-29T06:52:25Z

+		if err := doComment(failureBadgeURL + "\nUnable to run plan-preview. \ncause: " + err.Error()); err != nil {
+			return 1
+		}
+		log.Println(err)
+		return 1


This order?? err in doComment() is already printed in doComment(), and we want to print err of result, err := retrievePlanPreview( ??

(I'm not confident)

Suggested change

if err := doComment(failureBadgeURL + "\nUnable to run plan-preview. \ncause: " + err.Error()); err != nil {

return 1

}

log.Println(err)

return 1

log.Println(err)

if err := doComment(failureBadgeURL + "\nUnable to run plan-preview. \ncause: " + err.Error()); err != nil {

return 1

}

return 1

Yes, I want to print err from retrievePlanPreview.
The behavior differs when the doComment fails, my code doesn't print retrievePlanPreview's error, and your code does.
the doComment's error is shadowing the retrievePlanPreview's error and scope is narrow, so log.Println correctly prints err from retrievePlanPreview.

But I think it's better to print err from retrievePlanPreview even if doComment fails, so I'll apply your suggestion.

I changed doComment to return int.
Additionally, this commit enables logging err from retrievePlanPreview without changing the order, and it's clear to read. I think it's enough. WDYT?

7f14722

Thank you! it seems good!

t-kikuc · 2025-05-29T06:52:44Z

+			if err := doComment(failureBadgeURL + "\nUnable to run plan-preview. \ncause: " + err.Error()); err != nil {
+				return 1
+			}
+			log.Println(err)


Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

khanhtc1202

Look neat 👍

* Fix import grouping by make lint/go FIX=true Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix linter errors by make lint/go FIX=true Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Rewrite long if-else to switch Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Remove unnecessary zero-value initialization Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Remove unnecessary string conversion Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix to correctly call deferred functions log.Fatalf calls os.Exit(1) and deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix lint errors log.Fatal calls os.Exit, so the deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. This is a workaround to fix the lint errors. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix lint errors Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Initialize the slice with the correct capacity Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Change return type of doComment to int Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> --------- Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> Signed-off-by: pipecd-bot <pipecd.dev@gmail.com>

* Fix lint errors (#5897) * Fix import grouping by make lint/go FIX=true Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix linter errors by make lint/go FIX=true Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Rewrite long if-else to switch Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Remove unnecessary zero-value initialization Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Remove unnecessary string conversion Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix to correctly call deferred functions log.Fatalf calls os.Exit(1) and deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix lint errors log.Fatal calls os.Exit, so the deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. This is a workaround to fix the lint errors. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Fix lint errors Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Initialize the slice with the correct capacity Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> * Change return type of doComment to int Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> --------- Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> Signed-off-by: pipecd-bot <pipecd.dev@gmail.com> * Add 'title' to distinguish multiple projects in actions-plan-preview (#5905) * add title to args Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Add the title to filter latest comments Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Use hash Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * Fix tests Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> * add multibyte test Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> --------- Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> Signed-off-by: pipecd-bot <pipecd.dev@gmail.com> * Revert "Do treeless clone for git clone to improve performance (#5722)" (#5908) This reverts commit 7e74937. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> Signed-off-by: pipecd-bot <pipecd.dev@gmail.com> * Cut release v0.52.1 (#5909) Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Warashi <3600530+Warashi@users.noreply.github.com> Signed-off-by: pipecd-bot <pipecd.dev@gmail.com> --------- Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> Signed-off-by: pipecd-bot <pipecd.dev@gmail.com> Signed-off-by: t-kikuc <tkikuchi07f@gmail.com> Signed-off-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Shinnosuke Sawada-Dazai <shin@warashi.dev> Co-authored-by: Tetsuya KIKUCHI <97105818+t-kikuc@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Warashi <3600530+Warashi@users.noreply.github.com>

Warashi added 8 commits May 29, 2025 14:07

Fix import grouping by make lint/go FIX=true

8d6beb6

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Fix linter errors by make lint/go FIX=true

87f0e1e

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Rewrite long if-else to switch

7b3c845

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Remove unnecessary zero-value initialization

01841ee

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Remove unnecessary string conversion

b3dacd3

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Fix to correctly call deferred functions

acc9bd5

log.Fatalf calls os.Exit(1) and deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Fix lint errors

b87bf36

log.Fatal calls os.Exit, so the deferred functions are not called. This commit changes the _main function to return an integer and call os.Exit from main. This is a workaround to fix the lint errors. Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Fix lint errors

4430f56

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

github-actions Bot added area/go area/tool area/pipedv1 labels May 29, 2025

Warashi marked this pull request as ready for review May 29, 2025 05:46

Warashi requested a review from a team as a code owner May 29, 2025 05:46

Warashi requested review from ffjlabo, khanhtc1202 and t-kikuc May 29, 2025 05:46

khanhtc1202 reviewed May 29, 2025

View reviewed changes

Initialize the slice with the correct capacity

2465b2e

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

khanhtc1202 reviewed May 29, 2025

View reviewed changes

t-kikuc reviewed May 29, 2025

View reviewed changes

Change return type of doComment to int

7f14722

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

Warashi requested review from khanhtc1202 and t-kikuc May 29, 2025 07:15

Merge remote-tracking branch 'origin/master' into fix-lint-errors

d51b7bd

Signed-off-by: Shinnosuke Sawada-Dazai <shin@warashi.dev>

t-kikuc approved these changes May 29, 2025

View reviewed changes

t-kikuc enabled auto-merge (squash) May 29, 2025 08:20

khanhtc1202 approved these changes May 29, 2025

View reviewed changes

t-kikuc merged commit 6f5d4d8 into master May 29, 2025
45 of 47 checks passed

t-kikuc deleted the fix-lint-errors branch May 29, 2025 08:24

github-actions Bot mentioned this pull request Jun 2, 2025

[bot] Release v0.52.1 #5909

Merged

Warashi added cherry-pick v0.52.1 labels Jun 2, 2025

github-actions Bot mentioned this pull request Jun 2, 2025

Cherry-pick to release-v0.52.x #5911

Merged

github-actions Bot mentioned this pull request Jul 14, 2025

[bot] Release v0.52.2 #6027

Merged

github-actions Bot mentioned this pull request Aug 15, 2025

Release v0.53.0 #6123

Merged

	commits := make([]ReleaseCommit, 0)
	commits := make([]ReleaseCommit, 0, len(filteredCommits))

Conversation

Warashi commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Warashi May 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t-kikuc left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

khanhtc1202 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Warashi commented May 29, 2025 •

edited

Loading

codecov Bot commented May 29, 2025 •

edited

Loading

Warashi May 29, 2025 •

edited

Loading