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

fix(clean) report failures to remove item #5601

Merged
merged 6 commits into from
Oct 27, 2021

Conversation

jcassidyav
Copy link
Contributor

PR Checklist

What is the current behavior?

If ns clean failed to remove an item ( which is common on windows where the platforms directory is not removed properly), there was no indication to the user.

What is the new behavior?

Now the output of ns clean will indicate which item was not removed.
e.g.

_ Cleaning project...√ Cleaned directory hooks
× Failed to Clean directory platforms
√ Cleaned directory node_modules
√ Cleaned file package-lock.json
× Project unsuccessfully cleaned.

Fixes/Implements/Closes #5588.

@project-bot project-bot bot added this to Pull Request in CLI Team Oct 27, 2021
@cla-bot cla-bot bot added the cla: yes label Oct 27, 2021
Copy link
Member

@rigor789 rigor789 left a comment

Choose a reason for hiding this comment

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

Very nice improvement, I've definitely ran into the windows filesystem issues...

I've left a few minor suggestions for variable names, but overall LGTM! 👍

lib/commands/clean.ts Outdated Show resolved Hide resolved
lib/commands/clean.ts Outdated Show resolved Hide resolved
lib/services/project-cleanup-service.ts Outdated Show resolved Hide resolved
lib/services/project-cleanup-service.ts Outdated Show resolved Hide resolved
jcassidyav and others added 5 commits October 27, 2021 12:27
Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
Co-authored-by: Igor Randjelovic <rigor789@gmail.com>
@jcassidyav
Copy link
Contributor Author

@rigor789 thanks, I didn't realize my workspace wasn't linting, and I agree it is more readable for the variable names to be success.

@rigor789 rigor789 merged commit 3902257 into NativeScript:master Oct 27, 2021
CLI Team automation moved this from Pull Request to Done Oct 27, 2021
@vallemar
Copy link
Contributor

vallemar commented Nov 5, 2021

I applaud this PR, I would like to force the deletion of the platform folder. Some time ago someone commented in discord this script to eliminate the process that has it busy. Couldn't this be added before deleting this folder to ensure its deletion?

$process=(Get-WmiObject Win32_Process -Filter "name = 'java.exe' AND CommandLine like '%gradle%'")
if(-NOT ( $process  -EQ $Null)){
    echo Terminating $process.CommandLine
    $process.Terminate() | Out-Null
}

I say this because this behavior is a warming up of the head ...

@jcassidyav
Copy link
Contributor Author

@vallemar I posted that script to discord, but I think that would be a step too far to add to the cli, as who knows what people have running on their machines either now or in the future which would match that query.

A little later in discord I posted that from what I can see, this particular case is caused by The Gradle Daemon, the details on this process are documented here https://docs.gradle.org/current/userguide/gradle_daemon.html.

There are multiple options to turn it off, e.g. setting then environment variable documented here https://docs.gradle.org/current/userguide/gradle_daemon.html#daemon_faq
which prevents this happening for me.

@vallemar
Copy link
Contributor

vallemar commented Nov 7, 2021

@jcassidyav amazing. So if this can be disabled, do you think it would be okay to put something in the ns clean documentation? I'm also wondering if it would be okay to put a --force flag or something like that so that it launches the above query and closes the gradle process. What do you think? I do not know if they work with Windows in NS but it is a real nuisance

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
CLI Team
  
Done
Development

Successfully merging this pull request may close these issues.

ns clean windows shows that everything has gone well but does not delete the platforms folder
3 participants