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

danger local should consider uncommitted changes #972

Closed
sk- opened this issue Jan 13, 2020 · 8 comments
Closed

danger local should consider uncommitted changes #972

sk- opened this issue Jan 13, 2020 · 8 comments
Labels
You Can Do This This idea is well spec'd and ready for a PR

Comments

@sk-
Copy link
Contributor

sk- commented Jan 13, 2020

danger local only computes the differences between commits and does not consider changes not yet committed.

If I modify a file, I'd like to be able to locally check it before committing it, so that my local commits are compliant.

@orta
Copy link
Member

orta commented Jan 14, 2020

Yeah, I use it as a git commit hook - I'm open to a flag or to do some audits to decide if you want staged instead

@orta orta added the You Can Do This This idea is well spec'd and ready for a PR label Jan 14, 2020
@f-meloni
Copy link
Member

I think there was an initial implementation of it, but was not finished https://github.com/danger/danger-js/blob/master/source/commands/danger-local.ts#L14

@sk-
Copy link
Contributor Author

sk- commented Jan 15, 2020

I think that getting staged and unstaged filed is fairly easy, we just need to run git diff base instead of git diff base..HEAD.

For untracked files, we would need to change the way the list of created files are computed. Currently they are only obtained from the diff. However, given that there's no diff for those files they will never be there, and we would need to use git status.

Also, I'm not sure what the implications would be if we add this unstaged or untracked files. as
some other functions may fail.

WDYT?

@orta
Copy link
Member

orta commented Jan 16, 2020

I think it is probably worth faking the diff for the new files before it goes into the DSL generation, so that code can be consistent with danger pr and danger local (without --staging)

@Soyn
Copy link
Member

Soyn commented Feb 10, 2020

@orta Did you mean when run danger local(with no flag from stdin), the uncommitted changes would be as the diff by default?

@orta
Copy link
Member

orta commented Feb 10, 2020

No, I wanted it for the diff to master - so you can run danger locally before pushing your PRs

https://github.com/danger/danger-js/blob/master/package.json#L74

@Soyn
Copy link
Member

Soyn commented Feb 11, 2020

I'm a little confused on your comment. You mean we should add the uncommitted diff in current function.

const diff = await this.getGitDiff()

Soyn added a commit to Soyn/danger-js that referenced this issue Feb 11, 2020
@Soyn
Copy link
Member

Soyn commented Feb 11, 2020

I have sent a pull request about this issue. I am not sure this solution is the way you want. @orta

Soyn added a commit to Soyn/danger-js that referenced this issue Feb 17, 2020
@Soyn Soyn closed this as completed Feb 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
You Can Do This This idea is well spec'd and ready for a PR
Projects
None yet
Development

No branches or pull requests

4 participants