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

Add dev guide for pull request #6448

Merged
merged 1 commit into from
Apr 16, 2024
Merged

Add dev guide for pull request #6448

merged 1 commit into from
Apr 16, 2024

Conversation

BionIT
Copy link
Collaborator

@BionIT BionIT commented Apr 14, 2024

Description

This change adds a section in the dev guides about pull request, and it is based on observations when reviewing PRs, and the intent is to use current guide to illustrate the best practice and a reference to help improve the pull request quality. It might not capture everything but can be a good start.

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

Signed-off-by: Lu Yu <nluyu@amazon.com>

## Submit pull request
### Before submit a pull request
First-time contributors should head to the [contributing guide](https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/CONTRIBUTING.md) to get started.
Copy link
Member

Choose a reason for hiding this comment

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

Even for experience developer is it worth to read it again. Maybe we could add to PR check list.

If you don't know how to test a feature, please ask! Pull requests lacking sufficient testing coverage may be subject to delays in review or rejection until adequate tests are provided.

The repository has automated test workflows, and contributors submitting pull requests are required to check the failed test workflows and fix the tests related to their code change. If flaky test is identified, please ask a maintainer to retry the workflow.

Copy link
Member

Choose a reason for hiding this comment

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

should we add a link to https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/CONTRIBUTING.md#review-process
and also mention a little bit about backport process and responsibility

Copy link
Member

@zhongnansu zhongnansu left a comment

Choose a reason for hiding this comment

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

do we need to update change log?

@BionIT BionIT merged commit ce85706 into opensearch-project:main Apr 16, 2024
4 of 21 checks passed
opensearch-trigger-bot bot pushed a commit that referenced this pull request Apr 16, 2024
Signed-off-by: Lu Yu <nluyu@amazon.com>
(cherry picked from commit ce85706)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
BionIT pushed a commit that referenced this pull request Apr 16, 2024
(cherry picked from commit ce85706)

Signed-off-by: Lu Yu <nluyu@amazon.com>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants