diff --git a/assets/dismiss-review.png b/assets/dismiss-review.png new file mode 100644 index 0000000..056f80a Binary files /dev/null and b/assets/dismiss-review.png differ diff --git a/contributing/responding-pr-review.md b/contributing/responding-pr-review.md index 1dd0168..209af88 100644 --- a/contributing/responding-pr-review.md +++ b/contributing/responding-pr-review.md @@ -9,6 +9,9 @@ all observations but you should respond to all of them with a informative commen the reviewer knows what you did (addressed the changes, have questions about their observations, etc.) +It's okay and often faster to use Slack to resolve any disagreement, confusion, +or ambiguity and then update the thread once consensus is reached. + If a code change has been made, include a link to the changes so the reviewer can find them quickly. First, click on `Files changed` in your PR: @@ -30,9 +33,9 @@ Then, your response can be: [fixed](https://github.com/ploomber/jupysql/pull/787/files#diff-15ef0e119ce73b542976f499fcc3cbb967d30af8199e058aec2bc77c30973061R105-R114) ``` -## **Do not** mark the conversations as resolved +## **Do not** mark the conversations as resolved or dismiss reviews The reviewer is responsible for making conversations as resolved. ![](../assets/resolve-conversation.png) - +![](../assets/dismiss-review.png) diff --git a/contributing/submitting-pr.md b/contributing/submitting-pr.md index f24d1f9..78e2f7b 100644 --- a/contributing/submitting-pr.md +++ b/contributing/submitting-pr.md @@ -221,7 +221,7 @@ If the change breaks the API, the version will be handled case by case. However, ## Backwards compatibility -When breaking the API, we give heads up nnotice to our users so they have enough time to update their code. This involves showing warnings letting them know that a certain feature will be deprecated. +When breaking the API, we give heads up notice to our users so they have enough time to update their code. This involves showing warnings letting them know that a certain feature will be deprecated. We currently do not have a strict policy so we review cases on a case-by-case basis, but a good rule of thumb is to give at least a month's notice. This implies that Code Owners should ensure that the contributor opens a new PR with deprecation warnings, we merge the PR, and make a new release (by notifying Eduardo or Ido). This process should be prioritized so we make a release as soon as we decide that we'll break the API. @@ -392,4 +392,4 @@ Once you finished solving merge conflicts, do a quick check by looking at `Files Things to look for: - Are there changes I didn't do but that appear in the `Files changed`? If so, it means that something went wrong when merging conflicts -- Am I missing any changes? If so, you might've deleted some changes while solving merge conflicts \ No newline at end of file +- Am I missing any changes? If so, you might've deleted some changes while solving merge conflicts