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

feat: safe funcs, follow go template standards and sprout conventions #65

Merged
merged 43 commits into from
Sep 7, 2024

Conversation

42atomys
Copy link
Member

@42atomys 42atomys commented Aug 28, 2024

Description

This pull request updates all signatures to follow Go template standards and Sprout conventions, and add the safe behavior for functions instead of must as discussed on #32

Important

The signature changes do not introduce breaking changes. A deprecation notice will be displayed instead.
Two types of warn :

  1. Warn of renamed function :
WARN Template function `mustDateModify` is deprecated: please use `dateModify` instead
  1. Warn of signature changes:
WARN Template function `without` is deprecated: 
The signature of `without` has changed from `{{ without list 1 2 }}` to `{{ list | without 1 2 }}`,
please update your code before next upgrade.
This change will simplify the usage of the function and
respect go/template conventions and allow usage of pipe (`|`).

Note

This will spam the logger but will inform and prevent any breaking changes in the code. The signature migrations are encapsulated by banners in the codebase to avoid missing them when we remove the old signatures.

Documentation are available here : https://docs.atom.codes/sprout/~/changes/1eETtqCYtVeEPVLnbGs0?r=W7cRL8cXHka2XCuOcvBs

Checklist

  • I have read the CONTRIBUTING.md document.
  • My code follows the code style of this project.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • I have updated the documentation accordingly.
  • This change requires a change to the documentation on the website.

Additional Information

Registry Name Respect Function Signature Respect Pipe Syntax Notes
backward This registry are deprecated and cannot be migrated
checksum
conversion Deprecation of must functions
crypto This registry are deprecated and cannot be migrated
encoding Deprecation of must functions
env
_example
filesystem
maps Changes in signature parameters, Deprecation of must functions
numeric Changes in signature parameters, Deprecation of must functions
random
reflect Deprecation of must functions
regexp Deprecation of must functions
semver
slices Changes in signature parameters
std
strings
time Deprecation of must functions
uniqueid

@42atomys 42atomys added state/todo 🚀 This is confirmed, will work on soon domain/complicated 🟨 The relationship between cause and effect requires analysis or expertise aspect/dex 🤖 Concerns developers' experience with the codebase priority/medium 🟨 Priority 3 - Not blocking but should be fixed soon aspect/documentation 📚 Improvements or additions to documentation type/improvement ✨ Improvement to an existing feature labels Aug 28, 2024
@42atomys 42atomys added this to the v1.0 milestone Aug 28, 2024
@42atomys 42atomys self-assigned this Aug 28, 2024
@42atomys 42atomys changed the title chore: follow go template standards and sprout conventions chore: follow go template standards and sprout conventions [WIP] Aug 28, 2024
@42atomys 42atomys marked this pull request as ready for review September 2, 2024 11:22
@42atomys
Copy link
Member Author

42atomys commented Sep 2, 2024

@42atomys 42atomys changed the title feat: safe funcs, follow go template standards and sprout conventions [WIP] feat: safe funcs, follow go template standards and sprout conventions Sep 2, 2024
@42atomys 42atomys requested a review from a team September 2, 2024 12:46
Copy link

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

I only reviewed a part of your changes yet

error_test.go Outdated Show resolved Hide resolved
error_test.go Outdated Show resolved Hide resolved
handler.go Outdated Show resolved Hide resolved
handler.go Outdated Show resolved Hide resolved
handler_test.go Outdated Show resolved Hide resolved
@ccoVeille
Copy link

You should consider running testifylint on your codebase.

You can enable it via golangci-lint

error_test.go Outdated Show resolved Hide resolved
internal/runtime/safecall.go Outdated Show resolved Hide resolved
internal/runtime/safecall.go Outdated Show resolved Hide resolved
registry/conversion/conversion.go Show resolved Hide resolved
registry/encoding/encoding.go Show resolved Hide resolved
registry/maps/maps.go Show resolved Hide resolved
registry/reflect/reflect.go Show resolved Hide resolved
registry/regexp/regexp.go Show resolved Hide resolved
.golangci.yaml Outdated Show resolved Hide resolved
@42atomys
Copy link
Member Author

42atomys commented Sep 5, 2024

hi @ccoVeille, review returns done, one discussion stay open to let you read and react : #65 (comment)

Thanks for your time :)

Copy link

@ccoVeille ccoVeille left a comment

Choose a reason for hiding this comment

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

approved but a few suggestion anyway

internal/runtime/safecall_test.go Outdated Show resolved Hide resolved
Co-Authored-By: ccoVeille <3875889+ccoVeille@users.noreply.github.com>
@42atomys 42atomys enabled auto-merge (squash) September 7, 2024 22:46
@42atomys 42atomys merged commit 9d3cee5 into main Sep 7, 2024
15 checks passed
@42atomys 42atomys deleted the chore/follow-conventions branch September 7, 2024 22:46
Copy link

codecov bot commented Sep 7, 2024

Codecov Report

Attention: Patch coverage is 98.36957% with 12 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
registry/backward/functions.go 66.6% 3 Missing and 1 partial ⚠️
registry/encoding/functions.go 89.1% 4 Missing ⚠️
registry/random/functions.go 25.0% 2 Missing and 1 partial ⚠️
registry/backward/backward.go 0.0% 1 Missing ⚠️
Files with missing lines Coverage Δ
deprecated/deprecated.go 100.0% <100.0%> (ø)
error.go 100.0% <100.0%> (ø)
handler.go 100.0% <100.0%> (ø)
internal/runtime/safecall.go 100.0% <100.0%> (ø)
notice.go 100.0% <100.0%> (ø)
pesticide/rand_test_helpers.go 100.0% <100.0%> (ø)
pesticide/test_helpers.go 100.0% <100.0%> (ø)
registry/checksum/functions.go 100.0% <100.0%> (ø)
registry/conversion/conversion.go 92.0% <100.0%> (+2.0%) ⬆️
registry/conversion/functions.go 100.0% <100.0%> (ø)
... and 24 more

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect/dex 🤖 Concerns developers' experience with the codebase aspect/documentation 📚 Improvements or additions to documentation domain/complicated 🟨 The relationship between cause and effect requires analysis or expertise priority/medium 🟨 Priority 3 - Not blocking but should be fixed soon state/todo 🚀 This is confirmed, will work on soon type/feature ⭐ Addition of new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants