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

Faucet feature for SwankyNode #192

Merged
merged 11 commits into from
Feb 12, 2024
Merged

Faucet feature for SwankyNode #192

merged 11 commits into from
Feb 12, 2024

Conversation

prxgr4mm3r
Copy link
Collaborator

No description provided.

@prxgr4mm3r prxgr4mm3r self-assigned this Nov 15, 2023
@prxgr4mm3r prxgr4mm3r linked an issue Nov 17, 2023 that may be closed by this pull request
6 tasks
Copy link
Contributor

@ipapandinas ipapandinas left a comment

Choose a reason for hiding this comment

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

Nice idea the account balance command!

src/lib/substrate-api.ts Outdated Show resolved Hide resolved
src/commands/account/faucet.ts Outdated Show resolved Hide resolved
src/lib/substrate-api.ts Outdated Show resolved Hide resolved
src/lib/substrate-api.ts Outdated Show resolved Hide resolved
src/commands/account/faucet.ts Outdated Show resolved Hide resolved
src/commands/account/faucet.ts Outdated Show resolved Hide resolved
src/commands/account/faucet.ts Outdated Show resolved Hide resolved
src/commands/account/balance.ts Outdated Show resolved Hide resolved
src/commands/account/balance.ts Outdated Show resolved Hide resolved
@pmikolajczyk41 pmikolajczyk41 self-requested a review February 5, 2024 15:32
Copy link
Member

@pmikolajczyk41 pmikolajczyk41 left a comment

Choose a reason for hiding this comment

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

when running the balance command I get a bit vague error:

$ ../bin/run.js account balance
    TypeError: Cannot read properties of undefined (reading 'error')
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

if that's not a big problem, I would suggest to handle this, so that the user understands clearly that an argument is missing; similar thing touches faucet command

apart from this, works well for me

src/commands/account/create.ts Show resolved Hide resolved
src/lib/substrate-api.ts Show resolved Hide resolved
@ipapandinas ipapandinas changed the base branch from master to ink-devhub-1 February 12, 2024 18:27
@ipapandinas ipapandinas merged commit 0914aed into ink-devhub-1 Feb 12, 2024
3 checks passed
@ipapandinas ipapandinas deleted the feature/faucet branch February 12, 2024 20:18
@@ -0,0 +1,45 @@
import { Command } from "@oclif/core";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@ipapandinas Why is that class here? Isn't it more logical to put it into src/lib/..? When we use swanky account --help oclif thinks that it is a command.

Copy link
Contributor

Choose a reason for hiding this comment

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

The idea of having SwankyAccountCommand is to encapsulate common methods for account commands by inheriting this abstract class. But you are right we should move it, otherwise Oclif discovers it as a command.

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.

Add Faucet for Swanky Node
3 participants