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: Update Contribution Guidelines #1266

Conversation

VaishnaviHire
Copy link
Member

@VaishnaviHire VaishnaviHire commented Sep 27, 2024

Jira Issue: https://issues.redhat.com/browse/RHOAIENG-990

Description

This PR updates the contribution guidelines to reflect current working process of the team

How Has This Been Tested?

Screenshot or short clip

Merge criteria

  • You have read the contributors guide.
  • Commit messages are meaningful - have a clear and concise summary and detailed explanation of what was changed and why.
  • Pull Request contains a description of the solution, a link to the JIRA issue, and to any dependent or related Pull Request.
  • Testing instructions have been added in the PR body (for PRs involving changes that are not immediately obvious).
  • The developer has manually tested the changes and verified that the changes work

Copy link

openshift-ci bot commented Sep 27, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: asanzgom

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 15dfe1a into opendatahub-io:incubation Sep 27, 2024
4 checks passed
```
1. [linters](https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/.github/workflows/linter.yaml): Ensure the check for linters is successful. If it fails, run `make lint` to resolve errors
2. [api-docs](https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/.github/workflows/check-file-updates.yaml): Ensure the api-docs are updated when making changes to operator apis. If it fails, run `make generate manifests api-docs` to resolve errors
3. [unit-tests](https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/.github/workflows/unit-tests.yaml): Ensure unit tests pass. Run `make unit-tests`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
3. [unit-tests](https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/.github/workflows/unit-tests.yaml): Ensure unit tests pass. Run `make unit-tests`
3. [unit-test](https://github.com/opendatahub-io/opendatahub-operator/blob/incubation/.github/workflows/unit-tests.yaml): Ensure unit tests pass. Run `make unit-test`

1. **Link to Jira Issue**: Include the Jira issue link in your PR description.
2. **Description**: Provide a detailed description of the changes and what they fix or implement.
3. **Add Testing Steps**: Provide information on how the PR has been tested, and list out testing steps if any for reviewers.
4. **Review Request**: Tag the relevant maintainers(@opendatahub-io/odh-operator-maintainers ) or team members(@opendatahub-io/odh-platform-members) for a review.
Copy link
Member

Choose a reason for hiding this comment

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

for the one does not have permission to tag others in Reviewer, can tag in PR descripton or in comments?

4. Ensure your code passes `make lint` (we have a .golangci.yml file configured in the repo).
5. Ensure you write clear and concise comments, especially for exported functions.
6. Always check and handle errors appropriately. Avoid ignoring errors by using _.
7. Make sure to run `go mod tidy before` submitting a PR to ensure the `go.mod` and `go.sum` files are up-to-date.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
7. Make sure to run `go mod tidy before` submitting a PR to ensure the `go.mod` and `go.sum` files are up-to-date.
7. Make sure to run `go mod tidy` before submitting a PR to ensure the `go.mod` and `go.sum` files are up-to-date.

3. Use developer [guide](./README.md#developer-guide) to deploy operator [using OLM](./README.md#deployment) on a cluster.
4. Follow the steps given [here](./README.md#run-e2e-tests) to run e2e tests in your environment.

## Sync Changes in Downstream
Copy link
Member

Choose a reason for hiding this comment

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

do we need this section, if it is for community users


For general questions, feel free to open a discussion in our repository or communicate via:

- **Slack:** All issues related to ODH platform can be discussed in **#forum-openshift-ai-operator** channel.
Copy link
Member

Choose a reason for hiding this comment

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

should it be odh slack or internal slack?

sudo dnf install -y git-all
sudo dnf install -y golang
sudo dnf install -y podman
sudo dnf install -y cri-o kubernetes-kubeadm kubernetes-node kubernetes-client cri-tools
Copy link
Member

Choose a reason for hiding this comment

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

need this one?

```
$ cat local.mk
VERSION=9.9.9
IMAGE_TAG_BASE=quay.io/my-dev-env/opendatahub-operator
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
IMAGE_TAG_BASE=quay.io/my-dev-env/opendatahub-operator
IMAGE_TAG_BASE=quay.io/<user>/opendatahub-operator

Copy link

@adelton adelton left a comment

Choose a reason for hiding this comment

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

LGTM, added a couple of minor comments.

Lastly, add extra label for this issue if that can help us refine it.
1. **Check for Existing Issues:** Before creating a new issue, search the Jira project to see if a similar issue already exists.
2. **Create a Jira Ticket:** If the issue doesn’t exist, create a new ticket in Jira.
- **For Feature Requests:** Set the issue type to be `Initative`
Copy link

Choose a reason for hiding this comment

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

I always thought of Initiative as being a high-level important activity internal to the project or product. I'm a bit afraid of folks creating Initiatives for "trivial" stuff which will have user-visible results when implemented.

Could we just use Story for RFEs? We can of course always "upgrade" them to Initiative or Feature later.

4. Ensure your code passes `make lint` (we have a .golangci.yml file configured in the repo).
5. Ensure you write clear and concise comments, especially for exported functions.
6. Always check and handle errors appropriately. Avoid ignoring errors by using _.
7. Make sure to run `go mod tidy before` submitting a PR to ensure the `go.mod` and `go.sum` files are up-to-date.
Copy link

Choose a reason for hiding this comment

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

I also wonder -- do we have that go mod tidy in our PR checks (git grep does not find it but it might be executed some other way)? Or do we rely on folks being diligent with this?

Comment on lines +58 to +62
sudo dnf install -y zsh

# update PATH
echo 'export PATH=${PATH}:~/bin' >> ~/.zshrc
echo 'export GOPROXY=https://proxy.golang.org' >> ~/.zshrc
Copy link

Choose a reason for hiding this comment

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

What is zsh used for?

# upload public key to github

sudo dnf makecache --refresh
sudo dnf install -y git-all
Copy link

Choose a reason for hiding this comment

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

Wouldn't be git-core sufficient?


We follow the conventional commits format for writing commit messages. A good commit message should include:
1. **Type:** `fix`, `feat`, `docs`, `chore`, etc. **Note:** All commits except `chore` require an associated jira issue. Please add link to your jira issue.
Copy link
Contributor

Choose a reason for hiding this comment

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

when it became mandatory?

Lastly, add extra label for this issue if that can help us refine it.
1. **Check for Existing Issues:** Before creating a new issue, search the Jira project to see if a similar issue already exists.
2. **Create a Jira Ticket:** If the issue doesn’t exist, create a new ticket in Jira.
- **For Feature Requests:** Set the issue type to be `Initative`
Copy link
Member

Choose a reason for hiding this comment

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

I agree with @adelton that Initiative is internally used differently, so I would propose Story/Task, doesn't really matter, but we shouldn't go 2 levels higher (Initiative)

Comment on lines +27 to +30
1. **Link to Jira Issue**: Include the Jira issue link in your PR description.
2. **Description**: Provide a detailed description of the changes and what they fix or implement.
3. **Add Testing Steps**: Provide information on how the PR has been tested, and list out testing steps if any for reviewers.
4. **Review Request**: Tag the relevant maintainers(@opendatahub-io/odh-operator-maintainers ) or team members(@opendatahub-io/odh-platform-members) for a review.
Copy link
Member

Choose a reason for hiding this comment

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

We can mention the PR template here as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants