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

Feature/bulk pr for readme updates #174

Merged

Conversation

dennissivia
Copy link

This PR implements PRs #65, #74, #76.
Some of the PRs contain similar content and other are no longer
based on the current readme. This is an attempt to clean up the
readme state.

Copy link
Contributor

@junaruga junaruga left a comment

Choose a reason for hiding this comment

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

Thanks! It looks good to me!

@dennissivia
Copy link
Author

@junaruga Sorry I am not sure the squash merge was right. It looks as if that results in me being the author. We could revert the merge and I will re-do the merge without sqashing.

@junaruga
Copy link
Contributor

@scepticulous Ah in the case, the Squash Marge was not right. 3rd merge option (I forgot the name. The option is merging keeping original commits without merge commit.) was right.
But I am fine :)

@junaruga
Copy link
Contributor

@scepticulous do not worry. Maybe we don't have to revert it. I will write History file about original authors before next release.

@dennissivia
Copy link
Author

@junaruga thanks again. Quick question. Your preferred way for the workflow is squash+merge for "ones own" PR and regular merge for things like this, where somebody merges someone's else PR for whatever reason?

@junaruga
Copy link
Contributor

@scepticulous yes your recognition is correct.
Because I want to keep original commit as much as possible.

Some of the contributors do not like that original commit and their name has been removed when merging to master branch.
They may feel that they are not respected by rack-test project.
I have heard some 2 contributors complained about it.
And I can understand the feeling.

@dennissivia
Copy link
Author

@junaruga thanks for the update. I can also understand them and I think this process is fine for us as well. It would have been even less work for me in the first place.

@junaruga
Copy link
Contributor

@scepticulous I might find a better operation by chance in another project about how to merge someone's PR keeping the original commit.

Please look at below URL.

ruby/openssl#122

xxxxx changed the base branch to ruby:maint from ruby:master an hour ago

The upstream member was able to rebase my commit on another his branch.
So, maybe we can also do this way for our rack-test project.

@dennissivia dennissivia deleted the feature/bulk-pr-for-readme-updates branch May 17, 2017 10:34
@perlun
Copy link
Contributor

perlun commented May 17, 2017

@junaruga I don't think it's actually a rebase in that PR you referred to, but rather targeting which branch it will merge into.

@junaruga
Copy link
Contributor

@perlun oh okay. It's my mistake. Thanks for mentioning it.

@perlun
Copy link
Contributor

perlun commented May 19, 2017

Maybe you're right, I don't know actually. :)

alex-damian-negru pushed a commit to alex-damian-negru/rack-test that referenced this pull request Apr 5, 2021
* a small working example
* Little readme tweaks
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.

4 participants