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.

Sign up for GitHub

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

Jump to bottom

bugfix: add Props folder after running npm i #97

Merged
merged 3 commits into from
Dec 23, 2021
Merged

bugfix: add Props folder after running npm i #97

merged 3 commits into from
Dec 23, 2021

Conversation

jugshaurya
Copy link
Contributor

@jugshaurya jugshaurya commented Mar 25, 2021 •
edited
Loading

fixes: #96

Solution:-

  • added rm -rf scripts, so that netlify build and deploys the apps. (How I know? - checks were not passing on earlier commit because the react-native-elements folder was already there and hence not re-cloning, see Image below). Hence added two more execSync Scripts.
    Screenshot from 2021-03-25 13-28-03

  • Used execSync function

Updated-
Also, readme updated for the same, spaces is because of prettier.

also fixes issue #88

Tested How?

  • remove react-native-elements folder and Props folder (Do same if already there)
  • run npm i after that
  • both folders are coming up => Tested.

@netlify
Copy link

netlify bot commented Mar 25, 2021 •
edited
Loading

👷 Deploy Preview for rne-playground processing.

🔨 Explore the source changes: 33cd6f2

🔍 Inspect the deploy log: https://app.netlify.com/sites/rne-playground/deploys/61c497954d518f0007856b86

@jugshaurya jugshaurya force-pushed the bugfix/add-Props-folder branch 2 times, most recently from 70f6044 to 4886b5c Compare March 25, 2021 07:49
@jugshaurya jugshaurya changed the title bigfix: add Props folder after running npm i bugfix: add Props folder after running npm i Mar 25, 2021
@jugshaurya
Copy link
Contributor Author

@pranshuchittora, Please review

pranshuchittora
pranshuchittora reviewed Mar 26, 2021
View reviewed changes
injectLatestProps.js Outdated
exec(
const { execSync } = require("child_process");

execSync("rm -rf ./react-native-elements/", (err, stdout, stderr) => {
Copy link
Member

Choose a reason for hiding this comment

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

rm -rf is not recommended. Use rimraf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

sure.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pranshuchittora , added rimraf please review.

@jugshaurya jugshaurya mentioned this pull request Mar 26, 2021
Closed
pranshuchittora
pranshuchittora reviewed Apr 4, 2021
View reviewed changes
README.md Outdated
```sh
git clone https://github.com/react-native-elements/playground.git
cd playground
```
2. Install NPM packages

2. Install NPM packages, add Playground Props, react-native-elements folder
Copy link
Member

Choose a reason for hiding this comment

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

This change is not required

Copy link
Contributor Author

@jugshaurya jugshaurya Apr 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

ok, removed it

pranshuchittora
pranshuchittora reviewed Apr 4, 2021
View reviewed changes
injectLatestProps.js Outdated
exec(
const { execSync } = require("child_process");

execSync("rimraf ./react-native-elements/", (err, stdout, stderr) => {
Copy link
Member

@pranshuchittora pranshuchittora Apr 4, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Can you check the dir before deleting it?

Copy link
Contributor Author

@jugshaurya jugshaurya Apr 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

done using find-remove package npm

pranshuchittora
pranshuchittora reviewed Apr 4, 2021
View reviewed changes
injectLatestProps.js Outdated
exec("cp -r ./react-native-elements/website/docs/props ./src/content/Props"),

execSync(
"cp -r ./react-native-elements/website/docs/props ./src/content/Props",
Copy link
Member

Choose a reason for hiding this comment

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

This is UNIX based path can you use something like this -> https://www.npmjs.com/package/cp-file

Copy link
Contributor Author

@jugshaurya jugshaurya Apr 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

sure, but cp-file copies files, using copy-dir https://www.npmjs.com/package/copy-dir for doing it

@jugshaurya jugshaurya force-pushed the bugfix/add-Props-folder branch 5 times, most recently from a18c60d to 5fab498 Compare April 5, 2021 07:08
@jugshaurya
Copy link
Contributor Author

jugshaurya commented Apr 5, 2021 •
edited
Loading

@pranshuchittora please review
Made requested changes

  • checking folders before deleting using find-remove package npm
  • copying Props directory to src/content/Props via copy-dir package npm
  • adding console log to notify what is happening like shown in the image below
  • also added comments on what each operation is doing

Screenshot from 2021-04-05 12-40-57

@jugshaurya jugshaurya force-pushed the bugfix/add-Props-folder branch 3 times, most recently from 5e9d4aa to b75590b Compare April 6, 2021 17:16
@mhimanshu712
Copy link

@pranshuchittora I am having the same issue.
Do you approve these changes?

@jugshaurya jugshaurya mentioned this pull request Apr 16, 2021
Open
@jugshaurya jugshaurya mentioned this pull request Jun 20, 2021
Merged
2 tasks
@jugshaurya
Copy link
Contributor Author

jugshaurya commented Dec 23, 2021 •
edited
Loading

@flyingcircle Please review.

@jugshaurya jugshaurya mentioned this pull request Dec 23, 2021
Closed
@flyingcircle
Copy link
Contributor

You shoulddouble check that this still works. The props folder you copy from no longer exists.

@jugshaurya jugshaurya mentioned this pull request Dec 23, 2021
Closed
@jugshaurya
Copy link
Contributor Author

jugshaurya commented Dec 23, 2021 •
edited
Loading

You shoulddouble check that this still works. The props folder you copy from no longer exists.
@flyingcircle,yes you are right.
props folder is now the main folder, i will make the required changes soon.

@jugshaurya
Copy link
Contributor Author

@flyingcircle , changed folder from Props to main. Please check and review .

@flyingcircle flyingcircle merged commit abd7d89 into react-native-elements:master Dec 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pranshuchittora pranshuchittora Awaiting requested review from pranshuchittora

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

bug: injectProps.js not adding Props folder in content folder
4 participants
@jugshaurya @mhimanshu712 @flyingcircle @pranshuchittora