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

Throw error when building results in uncommitted changes #1

Merged

Conversation

kevinwilde
Copy link
Collaborator

If building a component library results in uncommitted changes, throw an error.

Currently, those uncommitted changes hang around, and then if a new commit tries to modify the same file, we get an error that the local changes would be overwritten by merge. I think it would be preferable to fail the original build so that the committer realizes they need to push the changes that result from building.

@connorjclark
Copy link
Collaborator

I recall running into this, I think when there was an auto-format fix flag enabled even in CI (I think I disabled the auto format in CI to fix that). Good idea to assert on this.

Would be good to add a diff of the unexpected changes to the error message. Other than that, it LGTM.

@@ -153,6 +153,11 @@ class DefaultBuilder implements Builder {
throw new Error(errors.join('\n====\n') || 'Unknown error while building')
})

if (await doPromiseExec('git status --short', { cwd: workingDir })) {
await doPromiseExec('git reset --hard && git clean -fd', { cwd: workingDir })
throw new Error(`Building ${componentLibrary} resulted in uncommitted changes`)
Copy link
Collaborator

Choose a reason for hiding this comment

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

"resulted in unexpected changes" would be more insightful.

@connorjclark connorjclark merged commit c888da1 into coursehero:master Nov 1, 2018
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.

2 participants