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

Add uncommited changeds to LocalGit #990

Merged
merged 4 commits into from
Feb 20, 2020

Conversation

Soyn
Copy link
Member

@Soyn Soyn commented Feb 11, 2020

@Soyn Soyn requested a review from orta February 11, 2020 14:28
Copy link
Member

@f-meloni f-meloni left a comment

Choose a reason for hiding this comment

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

I feel we shouldn't change how danger local works, but just add a parameter that you can pass to danger local to append in case the unstaged files to the diff

@orta
Copy link
Member

orta commented Feb 11, 2020

Yeah, there should already be a --staging flag in there this can use from when I first built this out

@Soyn Soyn force-pushed the fix/add_uncommitted_changes_to_local branch from ed0352f to 08be748 Compare February 17, 2020 12:59
@Soyn
Copy link
Member Author

Soyn commented Feb 17, 2020

I have updated the pull request.

@@ -15,6 +15,8 @@ export interface CliArgs {
dangerfile: string
/** So you can have many danger runs in one code review */
id: string
/** Use staged changes */
staged?: boolean
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure this works? The option above is .option("-s, --staging", "Just use staged changes.") - so I'd expect this to be staging?

Copy link
Member Author

Choose a reason for hiding this comment

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

In my local test. It works well. I will update the pull request later.

@Soyn
Copy link
Member Author

Soyn commented Feb 19, 2020

Make staged to staging

.option("-b, --base [branch_name]", "Use a different base branch")
setSharedArgs(program).parse(process.argv)

const app = (program as any) as App
const base = app.base || "master"

const localPlatform = new LocalGit({ base, staged: app.staging })
Copy link
Member

Choose a reason for hiding this comment

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

Oh, this is why it used to work - makes sense

@orta
Copy link
Member

orta commented Feb 20, 2020

Cool beans, thanks!

@orta orta merged commit cda717e into danger:master Feb 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants