Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

docs(CONTRIBUTING): add warning about forced push #13747

Closed
wants to merge 1 commit into from
Closed

docs(CONTRIBUTING): add warning about forced push #13747

wants to merge 1 commit into from

Conversation

dmitriz
Copy link
Contributor

@dmitriz dmitriz commented Jan 12, 2016

Add warning about the possible consequences of a forced push

docs(CONTRIBUTING): add warning about forced push

Replacing #13719

@petebacondarwin
Copy link
Contributor

Instead, I think we need to change the wording to be...

If we suggest changes then:

* Make the required updates.
* Re-run the Angular test suite to ensure tests are still passing.
* Commit your changes to the branch
* Push to your GitHub repository (this will update your Pull Request):

If the PR gets too outdated we may ask you to rebase and force push to update the PR:

git rebase master -i
git push origin my-fix-branch -f

@petebacondarwin petebacondarwin added this to the 1.5.x - migration-facilitation milestone Jan 12, 2016
@dmitriz
Copy link
Contributor Author

dmitriz commented Jan 12, 2016

So no warning?

What if a maintainer will ask to force push forgetting the consequences?

With an old PR especially this may lead to loss of people's work.

@gkalpak
Copy link
Member

gkalpak commented Jan 12, 2016

Without squashing, there shouldn't be any loss, right ?

@dmitriz
Copy link
Contributor Author

dmitriz commented Jan 12, 2016

Squashing is part of git rebase master -i

And things other than squashing might also affect it.

@petebacondarwin
Copy link
Contributor

OK, so you can add the warning as a line after what I wrote :-)

Add warning about the possible consequences of a forced push

docs(CONTRIBUTING): add warning about forced push
@petebacondarwin
Copy link
Contributor

Thanks for the changes - they are nearly right. I will fix up and merge.

petebacondarwin pushed a commit that referenced this pull request Jan 12, 2016
Add warning about the possible consequences of a forced push

Closes #13747
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants