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

chore: fixing outdated request-toolbox README #1099

Merged
merged 4 commits into from
May 16, 2023

Conversation

KolevDarko
Copy link
Contributor

@KolevDarko KolevDarko commented Apr 18, 2023

Renamed 'yarn request-toolbox' to the new 'yarn cli' in the README examples.

Update:

  • Added the troubleshooting trick from @yomarion.
  • Replaced all examples to be with: yarn request-toolbox for consistency
  • Explained that yarn cli also works

Copy link
Contributor

@benjlevesque benjlevesque left a comment

Choose a reason for hiding this comment

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

request-toolbox is valid, but requires the package to be built

@coveralls
Copy link

coveralls commented Apr 18, 2023

Coverage Status

Coverage: 87.342%. Remained the same when pulling 0a0c856 on chore/update-toolbox-docs into acce3ce on master.

@KolevDarko
Copy link
Contributor Author

I had all sorts of trouble with it. I couldn't get it to run even after building

@yomarion
Copy link
Member

I had all sorts of trouble with it. I couldn't get it to run even after building

I also had issues in the past but it is now working well. Not sure of this but: have you built the whole repo?

@KolevDarko
Copy link
Contributor Author

I had all sorts of trouble with it. I couldn't get it to run even after building

I also had issues in the past but it is now working well. Not sure of this but: have you built the whole repo?

Yes, I built the whole repo. I actually cloned the full repo from scratch and it still didn't work, perhaps you guys have an older build cache or something and that's why it works for you?
I'm suspecting it's been removed recently, @MantisClone have you tried this recently?

@alexandre-abrioux alexandre-abrioux removed their request for review April 18, 2023 16:03
@MantisClone
Copy link
Member

MantisClone commented Apr 18, 2023

  • ✔️ I cloned and built a fresh repo. And successfully ran the toolbox with yarn cli.
  • yarn request-toolbox and request-toolbox did not work after building the package. I believe it needs to be installed too.
  • 🤔 I tried a few things (yarn & npm from a few different directories) but couldn't figure out how to install the locally built toolbox package.
mantisclone@mantisclone-Inspiron-7620:~/projects/requestNetwork-cli/packages/toolbox$ yarn request-toolbox
yarn run v1.22.19
error Command "request-toolbox" not found.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
mantisclone@mantisclone-Inspiron-7620:~/projects/requestNetwork-cli/packages/toolbox$ request-toolbox
request-toolbox: command not found
mantisclone@mantisclone-Inspiron-7620:~/projects/requestNetwork-cli/packages/toolbox$ yarn cli
yarn run v1.22.19
$ cross-env NODE_ENV=development ts-node src/cli.ts
request-toolbox <command>

Commands:
  request-toolbox addAggregator <network>   adds a single aggregator
  request-toolbox addAggregators <network>  loads all known aggregators and adds
                                            missing
  request-toolbox currencyHash              Shows the currency hash of a
  <currencyCode>                            currency code
  request-toolbox deleteAggregator          remove a single aggregator
  <network>
  request-toolbox getConversionPath <from>  Shows the conversion path between 2
  <to>                                      currencies
  request-toolbox listAggregators           Helper for on-chain conversion
  [network]                                 administration
  request-toolbox listMissingAggregators    list missing aggregators for a given
  <list>                                    currency list
  request-toolbox hash submit <ipfsHash>    Forces the submission of an IPFS
                                            hash to the Request HashStorage
                                            contract
  request-toolbox calculateReference        Calculates the payment reference
  <requestId> <salt> <address>              from requestId, salt and address
  request-toolbox request create [amount]   Create a test request
  request-toolbox nonce                     Gets a wallet nonce
  request-toolbox transaction retry         Retries sending a pending
  <txHash>                                  transaction stuck in the mempool

Options:
  --help  Show help                                                    [boolean]

Not enough non-option arguments: got 0, need at least 1
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
mantisclone@mantisclone-Inspiron-7620:~/projects/requestNetwork-cli/packages/toolbox$ yarn cli request create
yarn run v1.22.19
$ cross-env NODE_ENV=development ts-node src/cli.ts request create
Create request with amount 1000
connect ECONNREFUSED 127.0.0.1:3000
Done in 3.36s.

@yomarion
Copy link
Member

@KolevDarko and @MantisClone

Try running yarn --check-files from the package root (maybe elsewhere works as well).

Need to document properly if this is correct, but:

$ yarn request-toolbox
# works
$ rm node_modules/.bin/request-toolbox
$ yarn request-toolbox
# error Command "request-toolbox" not found.
$ yarn --check-files
$ yarn request-toolbox
# works again

FYI, while playing around, I managed once to fix the issue with a simple yarn at the project root, but could not replicate.

Source: yarnpkg/yarn#3421
This seems to wrap it up:

Overall issue; Yarn should run the preinstall (and maybe other?) script(s) for each dependency before it tries to create the bin links for the dependency.

Copy link
Member

@yomarion yomarion left a comment

Choose a reason for hiding this comment

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

Instead we should document how to fix the build / preinstall.

@KolevDarko
Copy link
Contributor Author

yarn --check-files did the trick, I'll add a section in readme

@KolevDarko KolevDarko requested a review from yomarion May 16, 2023 14:47
packages/toolbox/README.md Outdated Show resolved Hide resolved
KolevDarko and others added 2 commits May 16, 2023 17:27
Co-authored-by: Yo <56731761+yomarion@users.noreply.github.com>
@KolevDarko KolevDarko merged commit 54e61d9 into master May 16, 2023
1 check passed
@KolevDarko KolevDarko deleted the chore/update-toolbox-docs branch May 16, 2023 15:43
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

5 participants