Skip to content

Conversation

@almas-x
Copy link
Contributor

@almas-x almas-x commented Jun 27, 2025

📑 Description

Closes goravel/goravel#702

This PR includes automated refactors applied via fieldalignment and modernize to improve code quality and consistency:

  • fieldalignment: optimized struct field order for better memory layout
  • modernize:
    • efaceany: replaced interface{} with Go 1.18+ any
    • minmax: simplified if-else assignments to built-in min/max
    • rangeint: converted classic for i := 0; i < n; i++ loops to for i := range n
    • fmtappendf, mapsloop, slicescontains: applied equivalent modernizations

✅ Checks

  • Added test cases for my code

Copilot AI review requested due to automatic review settings June 27, 2025 05:24
@almas-x almas-x requested a review from a team as a code owner June 27, 2025 05:24
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR applies automated refactors using fieldalignment and modernize to improve code consistency and leverage newer Go features.

  • Optimized struct field ordering for better memory layout.
  • Replaced interface{} with any and manual loops with slices.Contains, maps.Copy, and fmt.Appendf.
  • Simplified min/max logic and classic for i := 0; i < n; i++ loops using modern patterns.

Reviewed Changes

Copilot reviewed 86 out of 86 changed files in this pull request and generated no comments.

Show a summary per file
File Description
support/process/utils.go Replaced classic loop with for range 60.
foundation/application.go Replaced classic loop with for range 50.
database/db/utils.go Replaced for i := 0; i < length; i++ with for i := range length.
cache/memory_test.go Updated two for i := 0; i < 1000; i++ loops to for range 1000.
console/cli_helper.go Replaced manual loop bounds with for index := range lenShared.
Comments suppressed due to low confidence (6)

support/process/utils.go:26

  • In Go, you cannot for range over an integer literal. You should restore an index-based loop, e.g., for i := 0; i < 60; i++ {.
	for range 60 {

foundation/application.go:312

  • Ranging over an integer is invalid. Change back to a traditional loop or iterate over a slice: for i := 0; i < 50; i++ {.
		for range 50 {

database/db/utils.go:50

  • You cannot use range on an integer. Keep for i := 0; i < length; i++ { or range over a slice of that length.
		for i := range length {

cache/memory_test.go:75

  • for range cannot iterate an integer. Revert to for i := 0; i < 1000; i++ { for correct iteration.
	for range 1000 {

cache/memory_test.go:183

  • Invalid for range over an int. Use a conventional loop: for i := 0; i < 1000; i++ {.
	for range 1000 {

console/cli_helper.go:237

  • Cannot range over an int value. Either keep the original index loop or range over a slice/string: for index := 0; index < lenShared; index++ {.
	for index := range lenShared {

@codecov
Copy link

codecov bot commented Jun 27, 2025

Codecov Report

Attention: Patch coverage is 61.11111% with 14 lines in your changes missing coverage. Please review.

Project coverage is 71.16%. Comparing base (9719535) to head (93a8755).
Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
foundation/application.go 25.00% 6 Missing and 3 partials ⚠️
support/env/env.go 0.00% 2 Missing ⚠️
validation/validation.go 50.00% 1 Missing and 1 partial ⚠️
testing/http/test_request.go 0.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1096      +/-   ##
==========================================
+ Coverage   71.09%   71.16%   +0.06%     
==========================================
  Files         183      183              
  Lines       12854    12826      -28     
==========================================
- Hits         9138     9127      -11     
+ Misses       3345     3330      -15     
+ Partials      371      369       -2     

☔ 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.

Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Great PR 👍 Is there a script or github action to format files automatically?

@almas-x
Copy link
Contributor Author

almas-x commented Jun 27, 2025

Great PR 👍 Is there a script or github action to format files automatically?

It’s not easy to create a fully automated script for this, unfortunately.

The fieldalignment tool doesn’t exclude _test.go files by default, so some manual cleanup is required. And for modernize, the issues it detects and fixes can vary depending on the Go version, which means we also need to review and filter changes manually.

Because of that, it’s more practical to run these optimizations periodically—such as before major releases—rather than on every commit.

Copy link
Contributor

@hwbrzzl hwbrzzl left a comment

Choose a reason for hiding this comment

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

Great 👍

type ResponseImpl struct {
allowed bool
message string
allowed bool
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you add the sort rule to the PR description?

@almas-x almas-x merged commit b98d29f into master Jun 28, 2025
14 of 15 checks passed
@almas-x almas-x deleted the almas/#702 branch June 28, 2025 00:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Optimize Struct Field Alignment to Reduce Memory Usage

3 participants