Skip to content
This repository was archived by the owner on Mar 5, 2022. It is now read-only.

Conversation

@dmtrKovalenko
Copy link
Contributor

@dmtrKovalenko dmtrKovalenko commented Sep 17, 2020

TODO

  • review from Jess
@bahmutov
Copy link
Contributor

how do I run this?

init/init.ts Outdated
import NextTemplate from './templates/next'
import chalk from 'chalk'

const templates: Record<string, Template> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

if everything fails or in general we should give this URL for users to open for anything else https://github.com/bahmutov/cypress-react-unit-test#install

@dmtrKovalenko
Copy link
Contributor Author

npx tsnode init/init.ts
Also needs to add this to the bin of npm package

@bahmutov
Copy link
Contributor

can't we transpile it the same way we transpile code already? into something like bin folder (and yes, need to add bin alias to package.json file)

@dmtrKovalenko
Copy link
Contributor Author

dmtrKovalenko commented Sep 17, 2020

We transpile it this way. You can run yarn transpile and the assets will appear in the dist folder. Yes indeed I will change the folder.

@bahmutov
Copy link
Contributor

Yra, the CI is green

@dmtrKovalenko dmtrKovalenko marked this pull request as ready for review September 23, 2020 11:30
Copy link
Contributor

@bahmutov bahmutov left a comment

Choose a reason for hiding this comment

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

  • Add some kind of greeting line explaining the tool running, something like cypress-react-unit-test@version adding component testing wizard
  • when failing to find something we need to put link to the installation section of the README, otherwise the user has not idea what just happened
$ node ../cypress-react-unit-test/bin/init.js It looks like you did not install cypress. Can not find cypress.json in this or parent directories. 
  • we need to add this wizard to the README and push the current installation instructions into the recipes
  • checked if works for react-scripts, it works nicely
  • don't we need executable permission on bin/init.js file?
$ ls -la bin total 48 drwxr-xr-x 8 gleb staff 256 Sep 23 08:38 . drwxr-xr-x 34 gleb staff 1088 Sep 23 08:38 .. -rw-r--r-- 1 gleb staff 77 Sep 23 08:38 Template.js -rw-r--r-- 1 gleb staff 2249 Sep 23 08:38 findPackageJson.js -rw-r--r-- 1 gleb staff 7686 Sep 23 08:38 init.js drwxr-xr-x 8 gleb staff 256 Sep 23 08:38 templates -rw-r--r-- 1 gleb staff 1311 Sep 23 08:38 utils.js -rw-r--r-- 1 gleb staff 203 Sep 23 08:38 versions.js 
@dmtrKovalenko
Copy link
Contributor Author

Thanks for review!

when failing to find something we need to put link to the installation section of the README, otherwise the user has not idea what just happened

I saw your previous comment, and added a link to the https://github.com/bahmutov/cypress-react-unit-test/blob/main/docs/recipes.md because the #install section is only adding info about npm install (but if the user run this script he already installed the package right?) and points to te receips page.

@bahmutov
Copy link
Contributor

Ok I got this - we want the links to be shorter right. So I say we need to add to the README a section about the init command

## init You can use our command line wizard to give you instructions on configuring this plugin blah blah blah 

Then the link we can show from the CLI wizard could be https://github.com/bahmutov/cypress-react-unit-test#init (README anchors are working like https://github.com/bahmutov/cypress-react-unit-test#install) and in that section we can provide more info, explain things, link to the recipes. I think shorter links are much better

replace({ 'process.env.NODE_ENV': JSON.stringify('development') }),
```

See [examples/rollup](examples/rollup) folder for full example.
Copy link
Contributor

Choose a reason for hiding this comment

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

will come from PR 444

}

type FindPackageJsonResult =
| {
Copy link
Contributor

Choose a reason for hiding this comment

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

haha, we don't need no monads

Copy link
Contributor

@bahmutov bahmutov left a comment

Choose a reason for hiding this comment

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

still need the README init section

@bahmutov
Copy link
Contributor

anything remaining for this most excellent PR?

@dmtrKovalenko
Copy link
Contributor Author

Review from Jess ;)

@bahmutov bahmutov mentioned this pull request Sep 25, 2020
1 task
Copy link
Contributor

@JessicaSachs JessicaSachs left a comment

Choose a reason for hiding this comment

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

Almost perfect, great job. Thank you so much for your patience.

Co-authored-by: Jessica Sachs <jess@jessicasachs.io>
@dmtrKovalenko dmtrKovalenko merged commit 4fb01c0 into main Sep 29, 2020
@dmtrKovalenko dmtrKovalenko deleted the feature/smart-installation branch September 29, 2020 11:27
@bahmutov
Copy link
Contributor

🎉 This PR is included in version 4.15.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

4 participants