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: common checks #14

Merged
merged 8 commits into from
Sep 24, 2024
Merged

fix: common checks #14

merged 8 commits into from
Sep 24, 2024

Conversation

gauravlochab
Copy link
Collaborator

This has code cleanup and changes for common checks

@gauravlochab gauravlochab requested review from 0xArdi and dagacha and removed request for dagacha September 23, 2024 06:14
operate/cli.py Outdated

# type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why?

chain_id: str = "100",
chain_id: str = "100", # pylint: disable=unused-argument
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why disable and not remove instead?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed in next push

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm I still cannot see where the _chain_id is used in the method.


# type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why ignore the whole file?

Comment on lines 20 to 21
# type: ignore
"""This module implements the onchain manager."""
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why ignore the whole file?

@@ -650,7 +656,7 @@ async def _deploy_service_onchain(request: Request) -> JSONResponse:
operate.service_manager().deploy_service_onchain(
hash=request.path_params["service"]
)
operate.service_manager().stake_service_on_chain(
operate.service_manager().stake_service_on_chain( # pylint: disable=no-value-for-parameter
Copy link
Collaborator

Choose a reason for hiding this comment

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

The definition seems to require more args:

self, hash: str, chain_id: int, staking_program_id: str

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

the endpoint not used yet and the function in not implemented

@@ -17,6 +17,7 @@
# limitations under the License.
#
# ------------------------------------------------------------------------------
# type: ignore
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same question here.

@gauravlochab gauravlochab requested review from Adamantios and xvalex15 and removed request for 0xArdi and Adamantios September 23, 2024 18:03
@dagacha dagacha merged commit a1621ad into main Sep 24, 2024
3 checks passed
@Adamantios Adamantios deleted the fix/ci-checks branch September 24, 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