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

Adding a generic log function #1572

Merged
merged 21 commits into from
May 19, 2022
Merged

Adding a generic log function #1572

merged 21 commits into from
May 19, 2022

Conversation

nfurfaro
Copy link
Contributor

@nfurfaro nfurfaro commented May 17, 2022

This PR

  • uses the __is_reference_type and __size_of intrinsics to use either log or logd internally.
  • deprecates the type-specific log functions, ie: log_u64, log_b256, etc
  • Adds a logging module and reorgs the lib a little to bypass this blocker: Explicit ordering of dep statements is required to resolve dependencies. #409
  • preserves a bare chain module to prevent breaking everything that uses chain/auth (this feels a bit hacky... probably better to keep it clean, remove the chain mod and force users to update the path.)

close #1358
close #823

@nfurfaro nfurfaro added enhancement New feature or request lib: std Standard library labels May 17, 2022
@nfurfaro nfurfaro self-assigned this May 17, 2022
@nfurfaro nfurfaro requested review from adlerjohn and sezna May 17, 2022 16:24
@nfurfaro
Copy link
Contributor Author

I see that removing module chain altogether has caused some issues with imports.
working to fix that now.

@mohammadfawaz mohammadfawaz added the breaking May cause existing user code to break. Requires a minor or major release. label May 17, 2022
@nfurfaro nfurfaro marked this pull request as draft May 17, 2022 17:38
@nfurfaro
Copy link
Contributor Author

Ahh. I should really add more tests, there's only one existing atm :)

@nfurfaro nfurfaro marked this pull request as ready for review May 17, 2022 18:51
@nfurfaro nfurfaro marked this pull request as draft May 17, 2022 19:02
@nfurfaro
Copy link
Contributor Author

After #1573 lands I'll merge master re-push this.

sezna
sezna previously approved these changes May 18, 2022
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

LGTM!

@mohammadfawaz mohammadfawaz mentioned this pull request May 18, 2022
8 tasks
Conflicts:
	examples/hashing/src/main.sw
@nfurfaro nfurfaro marked this pull request as ready for review May 18, 2022 21:36
@nfurfaro nfurfaro requested review from sezna and removed request for sezna May 18, 2022 22:01
@nfurfaro nfurfaro merged commit 48d2740 into master May 19, 2022
@nfurfaro nfurfaro deleted the furnic/generic-log branch May 19, 2022 01:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking May cause existing user code to break. Requires a minor or major release. enhancement New feature or request lib: std Standard library
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Should logging be in it's own file in the std lib Add logd<T>(a: T) as a reporting/printing mechanism
4 participants