Docs/maintainers 266#274
Conversation
|
Contributors: @cristiandley, @rewop, please review this. After you reviewed, I'll merge this and add you to the contributor team. Thanks |
| * [CHANGELOG.md][CHANGELOG.md] is generated for the version as well | ||
| * `git tag` for the version is tagged to the commit too | ||
| * The last step is release the changes to `origin` and `npm`. | ||
|
|
There was a problem hiding this comment.
Issues
We encourage to report issues, the best way to do it is to write a good report. Some of items to set forth are:
- OS Platform (ie. Winx64/x86 7, MAC OS "El Capitan", Linux (Distro and Release).
- Node/Npm Version.
- Issue indeed (If possible the steps to get it).
There was a problem hiding this comment.
Agreed. We should have user document this details stuff down.
|
We could add an Issues section like i just commented on your PR. I've follow some issues here like "not running locally" that only says that... haha. Its a bit repetitive to ask for that always so that way we point "Please Follow Readme Issues section to report" Im glad you are open to add us @tomchentw !!! |
|
|
||
| Simple rules: | ||
|
|
||
| * Do rebase before merging. |
There was a problem hiding this comment.
@tomchentw Sorry for the silly question. Could you please clarify to me what you mean here? You mean rebase master into the branch?
Does this mean we don't merge master into the branch?
There was a problem hiding this comment.
Yes, we don't merge master into branch. We only do rebase the branch onto latest master.
There was a problem hiding this comment.
@tomchentw Sorry if I ask more questions, but I am not familiar with rebasing process to close a PR. My process usually uses git merge through the following three steps:
- merge alpha into branch
- resolve conflicts
- marge branch into master (github merge button. PR here gets closed).
Your flow instead doesn't need step 1 and 2, and instead of merging in step 3, we do a rebase. So we don't use the merge button from github. Am I correct?
There was a problem hiding this comment.
Yes. We don't use GitHub merge button for collaborator created PRs. Instead, we do:
- Rebase the branch to latest master so that it can be ff-merged.
- Force pushed the branch to let GitHub know the branch is updated.
- Merge the branch to master (should be fast-forward merge).
- Push master to GitHub (origin). GitHub will automatically close the PR :)
|
@cristiandley About the issues section, I came across react-bootstrap-datepicker contributing guidelines just today. I kind of like their simplicity and clarity in explaining it. Any other ideas? |
@WebRuin here.
I'll review it soon. |
|
@rewop do you mean the Issue Not Getting Attention? section? Or the whole |
|
@tomchentw I mainly mean the first three sections. They set guidelines about how to report bugs, or propose api changes. The statements are very clear and they create proper expectations. |
|
@rewop Got it. I really like their format. Would you like to fix it for me? :) Also, when committing the typo fixes that you just created, you should use |
|
@tomchentw Yes I can add it. About the commit, I was considering to squash (didn't know about fixup), but I wanted to wait for your comment :). Still getting used to it. |
rewop
force-pushed
the
docs/maintainers-266
branch
from
3534f40 to
824c44f
Compare
|
@rewop let me know after you've done the changes, thanks :) |
|
@tomchentw sorry if a little delayed on this. I didn't have time left after I fixed the commit. I will push the changes in the next few hours. |
rewop
force-pushed
the
docs/maintainers-266
branch
from
824c44f to
35a97eb
Compare
|
@tomchentw I made the changes, and also updated a little bit the structure and the language to improve understanding. Let me know if you spot any problem, I can make fixes by tomorrow. |
|
@rewop looks good! Thanks. Guess we can land this now? |
|
@tomchentw Yep!. Sorry my absence. Now im back. 👍 |
|
Sorry my absence too. I have been to a short holiday. Now I am back. I can take care of merging this one today. |
|
Thanks for merging this. Great work @rewop , @cristiandley . |
|
@idolize you may want to take a look at the contents of this PR. Feel free to ignore the discussions here. |
|
@cristiandley @rewop @idolize should we ask collaborators to review the PR before merging? |
|
Released v5.0.0. This version( |
3 participants
#266