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

chore: migrate the template engine to sprout #2006

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

42atomys
Copy link

This pull request includes the change discussed on issue #1638, primarily focused on updating dependencies, migrating from slim-sprig to sprout, and refactoring the templater functions.

Changes

  • Updated the Go version from 1.22.0 to 1.23.3 to ensure compatibility with Sprout.
  • Replaced github.com/go-task/slim-sprig/v3 with github.com/go-sprout/sprout.
  • Removed slim-sprig imports and added sprout and its registries.
  • Implemented backward compatibility for original sprig methods.
  • Moved templater functions to a new taskfunc registry to follow Sprout standard.
  • Added comprehensive tests for the new taskfunc package to ensure template functions are correctly tested.
  • Update the documentation to match sprout documentation.

Resolve #1638

@42atomys
Copy link
Author

Hi @andreynering, it's been a long time. I preferred to wait for the v1.0.0 release instead of implementing an unstable version, and now here we are.

I've reflected on all the changes and discovered a few points that need discussion 🙏

  1. Regarding the Go Task Functions, you've implemented a merge function that behaves like mergeOverride in Sprig. This implementation currently overrides the original merge behavior, making merge and mergeOverride functionally the same, although they're based on different libraries. To maintain compatibility, I added an alias to ensure the same behavior is retained, but this seems a bit strange to me:

    sprout.AddAlias(aliasMap, "mergeOverwrite", "merge")

    What would you prefer? Should we keep this alias and drop the packaged merge function? Or should we add a deprecation notice to clarify the difference and guide users to update their templates?

  2. I’ve included all functions from slim-sprig to define which registries to include. As discussed, the crypto package is excluded 😉. Additionally, semver and uuid are not included either. Let me know if you'd like fewer or more functions.

  3. I’ve followed your comments in the code to mark deprecated functions and added other deprecation rules to align with naming conventions:

    func (gtr *GoTaskRegistry) RegisterNotices(notices *[]sprout.FunctionNotice) error {
    sprout.AddNotice(notices, sprout.NewDeprecatedNotice("IsSH", "This function is deprecated. Consider removing it from your templates."))
    sprout.AddNotice(notices, sprout.NewDeprecatedNotice("FromSlash", "Use `fromSlash` instead."))
    sprout.AddNotice(notices, sprout.NewDeprecatedNotice("ToSlash", "Use `toSlash` instead."))
    sprout.AddNotice(notices, sprout.NewDeprecatedNotice("ExeExt", "Use `exeExt` instead."))
    sprout.AddNotice(notices, sprout.NewDeprecatedNotice("OS", "Use `os` instead."))
    sprout.AddNotice(notices, sprout.NewDeprecatedNotice("ARCH", "Use `arch` instead."))
    return nil
    }

Tip

Deprecation notices are non-breaking changes! They inform users about updates they need to make to their templates before old functions are removed. This ensures no breaking changes between versions. Sprout Notice Documentation

  1. While developing Sprout, I discovered several bugs in functions, including a few panics and memory errors. I’ve fixed them, so some functions might now behave differently (e.g., they returned a panic before but now return nil). This is just an informational note.

  2. To follow Go template conventions, all functions should be errorful. In Sprig, some functions have an error signature (any, error) while others don’t. Similarly, some have a Must version, and some don’t. In Sprout, we decided to enforce error signatures for consistency, meaning all functions return an error when something goes wrong.
    Sprout also provides the option for safe function versions to have a no-error signature (disabled by default). Let me know if you’d like to include this feature.

Thanks for your patience and for taking the time to answer me!

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

Successfully merging this pull request may close these issues.

Migrate from sprig (slim-sprig) to sprout?
1 participant