-
Notifications
You must be signed in to change notification settings - Fork 251
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
Switch log signature to str from bytes #366
Merged
Merged
Changes from all commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
a1b9148
Switch log signature to str from bytes
austinabell bb568c7
fix(log): switch macro type to expr to make api less restrictive
austinabell d00ef07
Switch function signature to &str to remove potential code duplication
austinabell 54252df
Merge branch 'master' into env/logsignature
austinabell 4994c92
Merge branch 'master' of github.com:near/near-sdk-rs into env/logsign…
austinabell 6af641c
Merge branch 'master' of github.com:near/near-sdk-rs into env/logsign…
austinabell ec98504
remove breaking change on function signature by adding log_str and up…
austinabell 84f714b
fix equivalent doc example
austinabell f514ec0
fmt
austinabell 844c3a6
Merge branch 'master' into env/logsignature
austinabell 3bd7cb1
add print added to log_str also
austinabell 2a37b9f
change log debug prints to std error instead of std out
austinabell a5c95b6
Merge branch 'master' into env/logsignature
mikedotexe d8d8109
Merge branch 'master' into env/logsignature
austinabell b695537
Merge branch 'master' into env/logsignature
austinabell 2ca4858
Merge branch 'master' into env/logsignature
mikedotexe 481d850
Merge branch 'master' of github.com:near/near-sdk-rs into env/logsign…
austinabell 59216bc
Merge branch 'env/logsignature' of github.com:austinabell/near-sdk-rs…
austinabell 2f0dd43
Merge branch 'master' into env/logsignature
austinabell 80c8f08
Merge branch 'master' into env/logsignature
austinabell 8473783
Merge branch 'master' into env/logsignature
austinabell File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should it be accepting anything that implements
ToString
trait? WDYT @matklad ?Btw, is this method going to support no-std?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see a huge benefit in supporting this because it would increase code size for the monomorphization of the generic and for more general usage,
log!
macro is used. I originally made it generic based onAsRef<str>
but since it added code size (tested and an example given above) the macro callsas_ref
and this base call remains as a concrete type. Having it beToString
would force an unnecessary allocation that I'm not sure can be optimized away by the compiler.As for no_std, yes it is no_std compatible and has the same memory layout as bytes. It's currently used as such in the no_std experimental SDK I've been playing with.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No,
ToString
would force allocation, and won't be enough to make a nice API. For the latter, we need a format-style macro.