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

docs: Add "Keeping Branch Up to Date and Resolving Merge Conflicts" section #123

Merged
merged 11 commits into from Oct 20, 2023

Conversation

adiati98
Copy link
Member

@adiati98 adiati98 commented Oct 11, 2023

Description

This PR adds the Keeping Branch Up to Date and Resolving Merge Conflicts section to walk through the process for this project.

Note for Reviewers

  • Because there is CLI involved for this project, screen recordings (GIFs) will be more clear than screenshots. So there will be GIFs (or video walkthrough) provided, but how to add them here is to be advised.

  • One of the steps to resolve conflict is to move contributor's profile details to the last list in the contributors array. This step is necessary because without it, the PRs that are awaiting for review have to update their branch locally and run npm run contributors:generate to regenerate the badge. It will be too complicated and discouraging for this simple project --> this is one of the reasons that the process won't be too clear with screenshots.

  • Once this PR is merged, there will be another PR following to add GIFs.

  • Videos walkthrough are planned to replace GIFs in the future.

What type of PR is this? (check all applicable)

  • πŸ• Feature
  • πŸ› Bug Fix
  • πŸ“ Documentation Update
  • 🎨 Style
  • πŸ§‘β€πŸ’» Code Refactor
  • πŸ”₯ Performance Improvements
  • βœ… Test
  • πŸ€– Build
  • πŸ” CI
  • πŸ“¦ Chore (Release)
  • ⏩ Revert

Related Tickets & Documents

Refers to #118

Mobile & Desktop Screenshots/Recordings

Added tests?

  • πŸ‘ yes
  • πŸ™… no, because they aren't needed
  • πŸ™‹ no, because I need help

Added to documentation?

  • πŸ“œ README.md
  • πŸ““ docs.opensauced.pizza
  • πŸ• dev.to/opensauced
  • πŸ“• storybook
  • πŸ™… no documentation needed

[optional] Are there any post-deployment tasks we need to perform?

[optional] What gif best describes this PR or how it makes you feel?

cross fingers GIF

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
Copy link
Member

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Hi @adiati98! :) The guide looks pretty good so far! :) I just gave some feedback in my inline comments. I hope you find them helpful! :)

@CBID2 CBID2 added the documentation Improvements or additions to documentation label Oct 11, 2023
@adiati98
Copy link
Member Author

Hi @adiati98! :) The guide looks pretty good so far! :) I just gave some feedback in my inline comments. I hope you find them helpful! :)

Thanks, @CBID2!
I left some comments and also added "Note for Reviewers" section in the PR description :)

Copy link
Member Author

@adiati98 adiati98 left a comment

Choose a reason for hiding this comment

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

@CBID2 I made some fixes for you to check out. Thank you!😊

README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Show resolved Hide resolved
Copy link
Member

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Looks good to me! :)

Copy link
Member

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

You got some merge conflicts on here that need solving @adiati98

@adiati98
Copy link
Member Author

You got some merge conflicts on here that need solving @adiati98

How ironic is that, @CBID2, because this PR is about resolving conflicts πŸ˜‚

@adiati98
Copy link
Member Author

Looks good to me! :)

Thanks! Now just need to discuss about the screen recordings before I make this ready for review ;)

@adiati98 adiati98 marked this pull request as ready for review October 13, 2023 11:15
@CBID2 CBID2 enabled auto-merge (squash) October 20, 2023 13:37
Copy link
Member

@BekahHW BekahHW left a comment

Choose a reason for hiding this comment

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

Although I think that adding screenshots are helpful, they aren't necessary especially if this remains in the readme. With the README, it's important that we don't overwhelm contributors with too much. I think that screenshots will start to make the README feel too long. I do have this blog post written about it that can be referenced here that has screenshots.

Depending on how things change here, we might want to add another folder in the repo for community-resources that includes translations and the info here, but I think for now, the repo is still manageable.

@adiati98
Copy link
Member Author

adiati98 commented Oct 20, 2023

@BekahHW this PR can't be merged because there were unresolved reviews from @CBID2. Now I closed them as resolved. You can try again.

If it doesn't work, you need to override @CBID2 review. I did this before in one PR, but forgot to take note.
I think there's an option to do so.
And at the end, you will get something like this:

dismissed stale


Update:

FYI @BekahHW @CBID2,
When there are multiple maintainers give reviews to PR, all of them have to approve the changes, individually.
If it happened that one hasn't reviewed, the PR cannot be merged.
So you either want to wait for it to be approved, or override it by:

  1. Click the requested change.
  2. Click three dots icon on the right side of the maintainer who request change.
  3. Click the "Dismiss review" button.

And don't forget to give brief description why you want to dismiss the review.

dismiss review

Copy link
Member

@CBID2 CBID2 left a comment

Choose a reason for hiding this comment

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

Looks great now @adiati98! :) Sorry for taking so long. I thought I approved your PR. I guess I did in my head lol πŸ˜‚

@CBID2 CBID2 merged commit 7b21edc into open-sauced:main Oct 20, 2023
@adiati98 adiati98 deleted the docs/add-resolve-conflict-section branch October 20, 2023 13:51
@CBID2 CBID2 linked an issue Oct 20, 2023 that may be closed by this pull request
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Feature: Add a Resolve Merge Conflicts section on README
3 participants