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

Option to dry run #6

Closed
aulisius opened this issue Feb 17, 2018 · 13 comments
Closed

Option to dry run #6

aulisius opened this issue Feb 17, 2018 · 13 comments
Labels
help wanted Extra attention is needed

Comments

@aulisius
Copy link
Contributor

Really useful tool! I think there should be a dry run option just to see what changes the tool will generate before applying them.

I would like to contribute on this, if you do consider such an option

Thanks,
Faizaan.

@hzoo
Copy link
Member

hzoo commented Feb 17, 2018

👍 I certainly understand why you'd want that but not sure how it would work exactly?

The output is just a change in the files so you would just do a git reset to go back and git diff to view? Unless you have something of an idea for it?

@aulisius
Copy link
Contributor Author

I was thinking, the output would actually be like a git diff. The only change is that it applies no changes. There isn't any special case I had in mind.

@hzoo
Copy link
Member

hzoo commented Feb 19, 2018

If you are already using git and run git diff after running the command, it is necessary to have a --dry-run option? The only thing you couldn't reset is if we automatically run npm install again which isn't implemented yet either and even that you could just run again.

@aulisius
Copy link
Contributor Author

I do agree with you. It's not really necessary

@hzoo
Copy link
Member

hzoo commented Feb 21, 2018

Unless we think a better default is --dry and to use --write for the actual run?

@aulisius
Copy link
Contributor Author

Well. That does sound interesting. I think this is something that needs to be discussed with more ppl. Some might prefer to go ahead with the change and some (like me) might want the dry run first before accepting changes.

@ljharb
Copy link
Member

ljharb commented Feb 22, 2018

Yes please make the default safe.

@aulisius aulisius reopened this Feb 22, 2018
@ljharb
Copy link
Member

ljharb commented Feb 22, 2018

Separately, having a dry run option that exits zero when no changes are made is always useful for CIs, to avoid having to manually check and reset a git diff.

@hzoo
Copy link
Member

hzoo commented Feb 22, 2018

I guess it depends what do you mean by safe? It modifies files in the git repo which can be reset. What's the usecase for running in CI other than if we figure out how to make it as a service to auto PR some repos?

@ljharb
Copy link
Member

ljharb commented Feb 22, 2018

I'm not sure off the top of my head :-) but in general, every tool I've ever used that modifies files, I've ended up needing a way to validate that the command has been ran in CI.

@hzoo hzoo added the help wanted Extra attention is needed label Feb 24, 2018
@aulisius
Copy link
Contributor Author

Can I take this up? @hzoo

@noahlemen
Copy link
Member

@aulisius are you still interested in working on this? feel free to start a PR to continue this conversation in if so! i think this would be useful for #52

@aulisius
Copy link
Contributor Author

Sure @kedromelon. Will be creating a PR by end of this week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

4 participants