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: nep330 build info field of contract metadata #1178

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

dj8yfo
Copy link
Contributor

@dj8yfo dj8yfo commented May 3, 2024

No description provided.

Copy link
Collaborator

@frol frol left a comment

Choose a reason for hiding this comment

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

This is the right direction! I have left a couple of review comments to address

FieldError::UnsetOrEmptyBuildCommand
);
let build_command =
build_command.split_whitespace().map(|st| st.to_string()).collect::<Vec<_>>();
Copy link
Collaborator

Choose a reason for hiding this comment

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

Ouch, split_whitespace is not something we want to take chances with. Let's find a lossless way of passing BUILD_COMMAND. JSON array of strings? So, basically, serde_json::from_str::<Vec<String>>(build_command)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeap, this does look like a great alternative to me)

near-sdk-macros/src/core_impl/contract_metadata/mod.rs Outdated Show resolved Hide resolved
}
env_field!(
self.link,
std::env::var("CARGO_NEAR_REPO_LINK_HINT").or(std::env::var("CARGO_PKG_REPOSITORY"))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Instead of CARGO_NEAR_* prefix, I would use NEP330_ prefix with the matching names of the fields:

NEP330_VERSION
NEP330_LINK
NEP330_BUILD_INFO_BUILD_ENVIRONMENT
...

This way we won't depend on cargo-near implementation detail. In this case, the naming should be dictated by near-sdk-rs.

dj8yfo added a commit to dj8yfo/near-sdk-rs that referenced this pull request Jun 6, 2024
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.

None yet

2 participants