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.

Sign up for GitHub

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

Jump to bottom

move stdlib: type_info::type_name() #2122

Merged
merged 3 commits into from
Jul 21, 2022
Merged

move stdlib: type_info::type_name() #2122

merged 3 commits into from
Jul 21, 2022

Conversation

msmouse
Copy link
Contributor

@msmouse msmouse commented Jul 21, 2022 •
edited by geekflyer
Loading

Description

Prepare for emitting

struct CreateTableEvent {
    handle: u128,
    key_type: String,
    value_type: String,
}

Test Plan

unittest

This change is Reviewable

@msmouse msmouse requested review from davidiw, wrwg and zekun000 July 21, 2022 05:50
davidiw
davidiw approved these changes Jul 21, 2022
View reviewed changes
Copy link
Contributor

@davidiw davidiw left a comment

Choose a reason for hiding this comment

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

thanks batman.

aptos-move/framework/tests/move_unit_test.rs
@@ -41,10 +41,15 @@ pub fn aptos_test_natives() -> NativeFunctionTable {
}

#[test]
fn move_unit_tests() {
Copy link
Contributor

Choose a reason for hiding this comment

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

wtf, your file name?

aptos-move/framework/aptos-stdlib/sources/type_info.move Outdated
@@ -18,6 +20,7 @@ module aptos_std::type_info {
}

public native fun type_of<T>(): TypeInfo;
public native fun full_name<T>(): string::String;
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe full_path? or just path?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wrwg opinion?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm... looks asymmetric. type_info::name_of<T>()?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

works for me and better than "path". originally I named it type_info::type_name()

@msmouse msmouse changed the title move stdlib: type_info::full_name() move stdlib: type_info::type_name() Jul 21, 2022
@msmouse msmouse force-pushed the 0720alden-fullname branch 2 times, most recently from cf856a1 to bd3e3e7 Compare July 21, 2022 17:01
@msmouse msmouse enabled auto-merge (squash) July 21, 2022 17:04
@github-actions
Copy link
Contributor

❌ Forge test failure

Forge is land-blocking

Forge test runner terminated

@github-actions
Copy link
Contributor

❌ Forge test failure

Forge is land-blocking

all up : 5448 TPS, 3131 ms latency, 6600 ms p99 latency,no expired txns

@github-actions
Copy link
Contributor

✅ Forge test success

Forge is land-blocking

all up : 5813 TPS, 2938 ms latency, 5750 ms p99 latency,no expired txns

@msmouse msmouse force-pushed the 0720alden-fullname branch from 53ce80a to e42ddd2 Compare July 21, 2022 21:36
@github-actions
Copy link
Contributor

✅ Forge test success

Forge is land-blocking

all up : 6187 TPS, 2739 ms latency, 3700 ms p99 latency,no expired txns

@msmouse msmouse merged commit 0518565 into main Jul 21, 2022
@msmouse msmouse deleted the 0720alden-fullname branch July 21, 2022 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidiw davidiw davidiw approved these changes

@wrwg wrwg Awaiting requested review from wrwg wrwg is a code owner

@zekun000 zekun000 Awaiting requested review from zekun000

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@msmouse @davidiw @wrwg