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

Add title to endpoint definition #108

Merged
merged 4 commits into from
Oct 15, 2024

Conversation

danielailie
Copy link
Contributor

No description provided.

Copy link

github-actions bot commented Oct 15, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  multiversx_sdk/abi
  abi_definition.py
Project Total  

This report was generated by python-coverage-comment-action

@danielailie danielailie self-assigned this Oct 15, 2024
andreibancioiu
andreibancioiu previously approved these changes Oct 15, 2024
@@ -86,14 +86,16 @@ def __init__(self,
inputs: List["ParameterDefinition"],
outputs: List["ParameterDefinition"],
payable_in_tokens: List[str],
only_owner: bool) -> None:
only_owner: bool,
title: str) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Tiny breaking chance, but should not be noticed by developers.

The alternative would have been to use Optional[str].

But it's all right this way, as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I updated to use the Optional[str] 👍

popenta
popenta previously approved these changes Oct 15, 2024
@danielailie danielailie dismissed stale reviews from popenta and andreibancioiu via 6e8bd59 October 15, 2024 08:07
@@ -7,7 +7,7 @@ allow-direct-references = true

[project]
name = "multiversx-sdk"
version = "0.13.2"
version = "0.14.0"
Copy link
Contributor

@andreibancioiu andreibancioiu Oct 15, 2024

Choose a reason for hiding this comment

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

If title is optional (no breaking change), can increment the last segment (since it's < v1).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will leave it like this as it is not a bug fix

@@ -87,7 +87,7 @@ def __init__(self,
outputs: List["ParameterDefinition"],
payable_in_tokens: List[str],
only_owner: bool,
title: str) -> None:
title: Optional[str]) -> None:
Copy link
Contributor

Choose a reason for hiding this comment

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

If optional, can be set to None by default. If None by default, then, in constructor, do self.title = title or "".

Otherwise, here it can also be title: str = "" (and no other change in constructor).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@danielailie danielailie merged commit ad12822 into main Oct 15, 2024
5 checks passed
@danielailie danielailie deleted the TOOL-273-add-title-to-abi-endpoints branch October 15, 2024 08:31
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.

3 participants