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

Fix scripts, make examples bin's #74

Merged
merged 8 commits into from
May 24, 2024

Conversation

Buckram123
Copy link
Contributor

@Buckram123 Buckram123 commented May 23, 2024

This PR fixes outstanding tasks on template

  • Move examples to bin
  • Update just commands to support multi-contracts
    • Update publish-schema command
    • Update publish command
  • Disable docs for adapter bins to avoid filename collision

@CyberHoward
Copy link
Contributor

Why did we change the dir name (app -> my-app) ?

@Buckram123
Copy link
Contributor Author

Buckram123 commented May 23, 2024

Why did we change the dir name (app -> my-app) ?

Just so package name matches to the directory name. And this section doesn't include renaming of the app/ https://github.com/AbstractSDK/app-template/blob/main/README.md#updating-names

Would you prefer to get it reverted?

echo "Please create metadata.json for module metadata"; exit; \
fi

tmp_dir="$(mktemp -d)"
schema_out_dir="$tmp_dir/{{namespace}}/{{name}}/{{version}}"
metadata_out_dir="$tmp_dir/{{namespace}}/{{name}}"
schema_out_dir="$tmp_dir/{{namespace}}/{{name}}/"
Copy link
Contributor

Choose a reason for hiding this comment

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

This schema out dir includes the namespace while the schema.sh script doesn't seem to do that. Is that the expected behaviour?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why would we need to include namespace in "app_template/schemas"? But we totally should include them when we push to the "AbstractSdk/schemas"

@CyberHoward
Copy link
Contributor

Oh and can we get rid of const_format as a deps? It's huge...

@Buckram123
Copy link
Contributor Author

Oh and can we get rid of const_format as a deps? It's huge...

Yeah we can remove it, we only use it in one place! What do you mean it's huge? Just tested compiled wasms with:

-rw-r--r--  448331 May 23 17:35 my_adapter.wasm
-rw-r--r--  424342 May 23 17:35 my_app.wasm

and without:

-rw-r--r--  448331 May 23 17:39 my_adapter.wasm
-rw-r--r--  424342 May 23 17:39 my_app.wasm

Could be nice to have abstract-based macro that generates those 3 consts - module-name, module-namespace and module-id and verify it in compile time.

@CyberHoward
Copy link
Contributor

Oh and can we get rid of const_format as a deps? It's huge...

Yeah we can remove it, we only use it in one place! What do you mean it's huge? Just tested compiled wasms with:

-rw-r--r--  448331 May 23 17:35 my_adapter.wasm
-rw-r--r--  424342 May 23 17:35 my_app.wasm

and without:

-rw-r--r--  448331 May 23 17:39 my_adapter.wasm
-rw-r--r--  424342 May 23 17:39 my_app.wasm

Could be nice to have abstract-based macro that generates those 3 consts - module-name, module-namespace and module-id and verify it in compile time.

Yes, would love a custom macro for this that expands in all three variables.

abstract_app::module_id!("my_namespace", "my-module")

Looked at the crate size and was very big. Guess compiler does a great job here at optimizing.

@Buckram123
Copy link
Contributor Author

Oh and can we get rid of const_format as a deps? It's huge...

Yeah we can remove it, we only use it in one place! What do you mean it's huge? Just tested compiled wasms with:

-rw-r--r--  448331 May 23 17:35 my_adapter.wasm
-rw-r--r--  424342 May 23 17:35 my_app.wasm

and without:

-rw-r--r--  448331 May 23 17:39 my_adapter.wasm
-rw-r--r--  424342 May 23 17:39 my_app.wasm

Could be nice to have abstract-based macro that generates those 3 consts - module-name, module-namespace and module-id and verify it in compile time.

Yes, would love a custom macro for this that expands in all three variables.

abstract_app::module_id!("my_namespace", "my-module")

Looked at the crate size and was very big. Guess compiler does a great job here at optimizing.

Don't think there's much to optimize! We only use one macro from the crate, so only it's result ends up in the binary(formatted str)

@CyberHoward CyberHoward merged commit 4c4d2aa into main May 24, 2024
11 checks passed
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.

2 participants