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: implement registry system (alias: loader) #46

Merged
merged 18 commits into from
Aug 14, 2024
Merged

Conversation

42atomys
Copy link
Member

@42atomys 42atomys commented Jul 22, 2024

Description

Following the discussion about function loading in #31, this huge refactor implement each method behind multiples registries.

Warning

This pull request are in progress, somes tests must be done before allowing it to be merge and release. Each feedback are welcome !

TODO

  • Allow registry to register aliases with function
  • Test new package
  • Provide detailed documentation for each module and glossary for handler and registry (WIP but available here)

Changes

  • Create the Handler and Registry interface
  • Define the rules of a registry
  • Add a Readme about how to create a registry
  • Migrate all existing functions into separated registry
  • Backward compatibility function FuncsMap register all registry by default

Fixes #10

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

This update following the discussions #31

@42atomys 42atomys self-assigned this Jul 22, 2024
@42atomys 42atomys added aspect/depencencies 📦️ Concerns dependencies of the project type/feature ⭐ Addition of new feature aspect/dex 🤖 Concerns developers' experience with the codebase aspect/documentation 📚 Improvements or additions to documentation labels Jul 22, 2024
@42atomys 42atomys added this to the v1.0 milestone Jul 22, 2024
@42atomys
Copy link
Member Author

First self feedback: allow registry to register aliases with functions

@42atomys
Copy link
Member Author

42atomys commented Aug 2, 2024

Second step :

  • test new package
  • provide detailled documentation for each module and glossary for handler and registry

@42atomys 42atomys force-pushed the feat/registry-system branch from 95d6990 to bc3fb90 Compare August 6, 2024 19:12
@42atomys
Copy link
Member Author

42atomys commented Aug 11, 2024

Currently start the last step :

  • Write a detailed documentation for each registry, a getting started and explanations for handler and registry, this will be a huge work to import it on Gitbook. The in progress documentation are available here.

🎉 This pull request are done in case of functionalities and tests. This will be merged soon as the documentation are ready too

handler_test.go Outdated Show resolved Hide resolved
handler_test.go Outdated Show resolved Hide resolved
handler_test.go Outdated Show resolved Hide resolved
internal/helpers/helpers_test.go Show resolved Hide resolved
internal/helpers/helpers_test.go Outdated Show resolved Hide resolved
internal/helpers/helpers_test.go Show resolved Hide resolved
pesticide/test_helpers.go Outdated Show resolved Hide resolved
pesticide/test_helpers.go Outdated Show resolved Hide resolved
registry/checksum/functions.go Outdated Show resolved Hide resolved
registry/checksum/functions.go Outdated Show resolved Hide resolved
registry/crypto/functions.go Show resolved Hide resolved
registry/crypto/functions.go Show resolved Hide resolved
registry/crypto/helpers.go Show resolved Hide resolved
registry/crypto/helpers.go Show resolved Hide resolved
registry/crypto/helpers.go Show resolved Hide resolved
@42atomys
Copy link
Member Author

Hi @ccoVeille, thanks for your contribution in reviewing this big PR 💜

According to multiple comments on the crypto package, this package was backported from sprig and will be dropped in a few versions due to significant security issues.

I've been focusing on rewriting and enhancing other registries, so I haven't made any changes to this one.

@ccoVeille
Copy link
Contributor

Old code is not funny. Noted

@caarlos0
Copy link

it looks really good! I liked the registry idea, and the implementation looks good!

lgtm!

@42atomys
Copy link
Member Author

Thanks @ccoVeille and @caarlos0 for your time and your contribution to reviewing this next step of Sprout 🌱 💜

@42atomys 42atomys merged commit 170e91b into main Aug 14, 2024
14 checks passed
@42atomys 42atomys deleted the feat/registry-system branch August 14, 2024 20:11
Copy link

codecov bot commented Aug 14, 2024

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
aspect/depencencies 📦️ Concerns dependencies of the project aspect/dex 🤖 Concerns developers' experience with the codebase aspect/documentation 📚 Improvements or additions to documentation type/feature ⭐ Addition of new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🌱 functions loading: create a complete solution to load specific function
3 participants