pmadruga / react-native-clean-project

Automating the clean up of a React Native project
MIT License
1.21k stars 53 forks source link

feat: add android clean project #38

Closed wilau2 closed 4 years ago

mikehardy commented 4 years ago

Hey there! Out of curiosity, is this closed for a specific reason? The related issue is correct in that deleting android/build is not sufficient, it should be android/app/build but even then that is just convention, the 'app', does not have to be called 'app' so that folder might not exist, meaning ./gradlew clean is actually the right approach

so it seems like this would be a good change, in my opinion

wilau2 commented 4 years ago

Hey @mikehardy this one is still open: https://github.com/pmadruga/react-native-clean-project/issues/36

mikehardy commented 4 years ago

@pmadruga near as I can tell, this is a good change and a correct implementation of what should be done to clean android. No idea why the PR is closed, I think it should be merged :-)

pmadruga commented 4 years ago

Sure @mikehardy. Just wanted to see the reason why it was closed (maybe the author found something fishy?).

mikehardy commented 4 years ago

Maybe with the fresh comments @wilau2 will chime in :-)

wilau2 commented 4 years ago

Hey guys, I ended up creating scripts in packages.json directly. It was easier for me to modify. At this point in our project, this is what we have:

"clean": "npm run clean:ios && npm run clean:android && npm run clean:javascript && rm -rf node_modules",
"clean:android": "lerna exec -- 'if [ -d \"android\" ]; then (cd android && ./gradlew clean); fi'",
"clean:ios": " lerna exec -- rm -rf ios/build && lerna exec -- rm -rf ios/pods",
"clean:javascript": "lerna clean --yes && rm -rf $TMPDIR/react-* && rm -rf $TMPDIR/metro-*",
mikehardy commented 4 years ago

Ah cool @wilau2 - thanks for responding. From my perspective then, this is still a desired react-native-clean-project feature, and if I were to implement it, I'd implement it exactly the same. Any reason not to keep the PR open and mere it for others to use so they don't have to do the package.json scripting if they don't want to? I use react-native-clean-project in my role as maintainer of a few repos so I can give people one command to clean out completely so it's valuable to me to have it all here in one spot

wilau2 commented 4 years ago

I moved away and you can probably see my train of thoughts with the different PRs I closed:

image

In the end, I'd prefer a collection of reusable scripts than a dynamic client. I needed to fight to make it command line friendly from our ci. Make it support npm instead of yarn and yadi yada. 🤷

This PR is probably good to merge, I was just blocked by other stuff I needed to modify and got lost in a lot of PRs at some point.

pmadruga commented 4 years ago

That's really interesting and thanks for sharing it. I wonder whether a simple Q&A would work? For example, when running the library for the first time, it would ask whether is android/ios, npm/yarn and then generate a file with the config options. Next time the script runs, it would read from that file.

mikehardy commented 4 years ago

For me this project is specifically about wiping away corrupt state / caching in a deterministic never-fails way (so I can converge colleague's environments with one command, and know that CI / release builds work every time). So for it to have it's own state across runs is like giving a fish a bicycle :-), completely outside what I'm interested in - likely to add another set of quirks to the one tool I trust to de-quirk everything else

wilau2 commented 4 years ago

Requirements:

I prefer it to be long but have 100% success rate than customizable to save some time.

pmadruga commented 4 years ago

That's great feedback, folks. Thank you for it.

I'll be looking into incorporating the PR into master.

PS: npm run clean already includes npm install. But now that I'm thinking, maybe it should not include npm install (hence avoiding the whole yarn VS npm issue).

mikehardy commented 4 years ago

sorry for the very late feedback (but thank you for merging this!) - you mention:

PS: npm run clean already includes npm install. But now that I'm thinking, maybe it should not include npm install (hence avoiding the whole yarn VS npm issue).

I have a preference for "everything by default" but "everything with a switch" as stated before - just a personal preference, I understand people are different - but based on that, having it be one thing that "cleans node_modules" and one separate thing "installs node_modules" would be more towards my liking.

A minor point for me though, I still recommend this project about 2x a day :-) - it has 265 stars now too, pretty cool I think. Cheers