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

[WIP] added contributing.md file to assist new contributors #946

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
213 changes: 213 additions & 0 deletions docs/CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,213 @@
# Contributing to Devworkspace-Operator
Copy link
Collaborator

Choose a reason for hiding this comment

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

For consistency with the README, the project should be referred to as "DevWorkspace Operator". @amisevsk correct me if I am wrong.


Hello there! thank you for choosing to the contributing to devfile/devworkspace-operator. Navigate through the following to understand more about contributing here.
kernelpanic77 marked this conversation as resolved.
Show resolved Hide resolved

- [Contributing to Devworkspace-Operator](#contributing-to-devworkspace-operator)
kernelpanic77 marked this conversation as resolved.
Show resolved Hide resolved
- [Before You Get Started](#before-you-get-started)
- [Code of Conduct](#code-of-conduct)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure if you had ideas for the "For Newcomers" section, but if you end up omitting it, it may be worth moving the code of conduct section to the end of this document (or at least, after the "How to Contribute" section).

This is just so that the contributing instructions are immediately visible (as the Code of Conduct section could be a bit lengthy).

Also, if you're looking for inspiration for a code of conduct, here's one

Copy link
Collaborator

Choose a reason for hiding this comment

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

Actually, here's an even better Code of Conduct from the devfile API repo: https://github.com/devfile/api/blob/main/CODE_OF_CONDUCT.md. It's probably worth making another, separate file for the code of conduct and linking to it in the CONTRIBUTING.md

Copy link
Author

Choose a reason for hiding this comment

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

I am actually omitting the newcomer's section. As it just doesn't seem an immediate requirement for now.

- [For Newcomers](#for-newcomers)
- [How to Contribute](#how-to-contribute)
- [Set up your Local Development Environment](#set-up-your-local-development-environment)
- [Testing Changes](#testing-changes)
- [Signing-off on Commits](#signing-off-on-commits)
(#testing-your-changes)
- [Signing-off on Commits](#signing-off-on-commits)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Accidentally duplicated lines?

Copy link
Author

Choose a reason for hiding this comment

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

Yes, updated.


# Before You Get Started

## Code of Conduct

## For Newcomers

# How to Contribute

<!--
AObuchow marked this conversation as resolved.
Show resolved Hide resolved
## Prerequisites

Make sure you have the following prerequisites installed on your operating system before you start contributing:

- [Nodejs and npm](https://nodejs.org/en/)

To verify run:

```
node -v
```

```
npm -v
```

- [Gatsby.js](https://www.gatsbyjs.com/)

To verify run:

```
gatsby --version
```

**Note:** If you're on a _Windows environment_ then it is highly recommended that you install [Windows Subsystem for Linux (WSL)](https://docs.microsoft.com/en-us/windows/wsl/install) both for performance and ease of use. Refer to the [documentation](https://docs.microsoft.com/en-us/windows/dev-environment/javascript/gatsby-on-wsl) for the installation of _Gatsby.js on WSL_. -->

## Set up your Local Development Environment

Follow the following instructions to start contributing.

**1.** Fork [this](https://github.com/devfile/devworkspace-operator) repository.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Very minor, but I would change this to Fork the [DevWorkspace Operator](https://github.com/devfile/devworkspace-operator) repository

Copy link
Author

@kernelpanic77 kernelpanic77 Jan 24, 2023

Choose a reason for hiding this comment

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

@kernelpanic77 looking great so far sunglasses In addition to @amisevsk's review, left some minor comments and potential suggestions.

After these comments are addressed, it might be worth adding a section on developing the webhook. IIRC, this process involves:

Thanks @AObuchow. This makes sense, I'll add a specific sub section for testing and developing webhooks in the "testing your changes" section.

  1. Make a change to the webhook
  2. Ensure the DWO_IMG environment variable points to your container image repository, eg. export DWO_IMG=quay.io/aobuchow/dwo-webhook:next
  3. Run make docker restart (assuming DWO is already deployed to the cluster, otherwise make docker install)
  4. Wait for the webhook deployment to update with your image that contains your latest changes.

There may be a better way to develop the webhook, but this is generally the flow that I've performed.

I have not contributed to developing webhooks yet 😅 , so this looks right to me. @amisevsk @ibuziuk would you like to add any steps to the process specified by @AObuchow.


**2.** Clone your forked copy of the project.

```
git clone https://github.com/<your-github-username>/devworkspace-operator.git
```

**3.** Navigate to the project directory.

```
cd devworkspace-operator
```

**4.** Add a reference(remote) to the original repository.

```
git remote add upstream https://github.com/devfile/devworkspace-operator.git
```

**5.** Check the remotes for this repository.

```
git remote -v
```

**6.** Always take a pull from the upstream repository to your master branch to keep it at par with the main project (updated repository).
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should avoid focusing on how to set up git repositories for the contributing guide -- assume the user has cloned a suitable repo to their local machine and wants to begin writing code for it. To this end, focus on

  1. What's required
  2. What the flow is for starting/debugging/testing

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I have added a "testing your changes" section for describing the general workflow of debugging and testing.


```
git pull upstream master
```

**7.** Create a new branch.

```
git checkout -b <your_branch_name>
```

**8.** Start the minikube cluster.
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should probably be scoped to a "developing on minikube" section; we don't require developers use minikube. In addition, how to start/configure minikube is out of scope for this article except where required (e.g. if we needed a specific amount of memory)

Copy link
Author

Choose a reason for hiding this comment

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

Hi @amisevsk! I have added that the steps are specifically for developing the devworkspace-operator on a minikube cluster. Should I add anything else ?


```
minikube start
```

**9.** Enable the ingress add-on for your minikube cluster.

```
minikube addons enable ingress
```

**10.** Set the namespace environment variable for the development environment to avoid changes inside the default namespace.

```
export NAMESPACE="devworkspace-controller"
```

**11.** Install the kubernetes certificate management controller to generate and manage TLS certificates for your cluster.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's worth noting that the cert manager is required only for Kubernetes, so if a "developing on minikube" section of this documented is created, this should probably be specific to it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Cert-manager is required for all deployments on Kubernetes, not just minikube.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, adding this comment in the steps. Thanks @AObuchow @amisevsk.


```
make install cert-manager.
Copy link
Collaborator

Choose a reason for hiding this comment

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

I've always used make install_cert_manager as this is the rule defined here and also referenced in the README.

Though, make install cert-manager does seem to work? Not sure where this recipe is defined as I couldn't find it when grepping the repo.

Copy link
Author

Choose a reason for hiding this comment

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

yes, making the change. make install_cert_manager seems right to me as well.

```

**12.** Install the dependencies for running the devworkspace-operator in your cluster.

```
make install
```

**13.** Scale down the replicas of pods to 0.
Copy link
Collaborator

@AObuchow AObuchow Oct 7, 2022

Choose a reason for hiding this comment

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

Maybe "Scale down the controller manager deployment's pods to 0."?

The reason this step is necessary is that only 1 controller manager can be run at a time (otherwise their operations will conflict/"fight" with each other), and if we are running the controller locally then we need to ensure the controller is not running on the cluster.

Copy link
Author

Choose a reason for hiding this comment

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

Noted 👍 .


```
kubectl patch deployment/devworkspace-controller-manager --patch "{\"spec\":{\"replicas\":0}}" -n $NAMESPACE
```

**14.** Run the devworkspace-operator.

```
make run
```
Copy link
Collaborator

Choose a reason for hiding this comment

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

Structurally, this should appear in a section like "running the controller locally"; this is not the only way to contribute so we shouldn't present it as such.


This will run the devworkspace-controller in your local cluster (minikube/openshift).
Copy link
Collaborator

Choose a reason for hiding this comment

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

Technically, this will make the devworkspace-controller run locally on your system (not on your local cluster).
I would remove the mention of "(minikube/openshift)" and, as @amisevsk mentioned, make this appear in a section on running the controller locally.

Copy link
Author

Choose a reason for hiding this comment

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

Noted 👍 .


**15.** Make your changes in the new branch and trach the changes.

```
git add .
```

**16.** Commit your changes. To contribute to this project, you must agree to the [Developer Certificate of Origin (DCO)](#signing-off-on-commits) for each commit you make.

```
git commit --signoff -m "<commit subject>"
```

or you could go with the shorter format for the same, as shown below.

```
git commit -s -m "<commit subject>"
```

**17.** While you are working on your branch, other developers may update the `master` branch with their branch. This action means your branch is now out of date with the `master` branch and missing content. So to fetch the new changes, follow along:
Copy link
Collaborator

Choose a reason for hiding this comment

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

As @amisevsk mentioned, I would omit instructions on setting up git/developing with git. With that being said, for DWO the master branch is called main.

Copy link
Author

Choose a reason for hiding this comment

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

Yes, I have removed the git/developing instructions for commit instructions.


```
git checkout master
git fetch origin master
git merge upstream/master
git push origin
```

Now you need to merge the `master` branch into your branch. This can be done in the following way:

```
git checkout <your_branch_name>
git merge master
```

**18.** Push the committed changes in your feature branch to your remote repo.

```
git push -u origin <your_branch_name>
```

## Testing Changes

## Signing-off on Commits

To contribute to this project, you must agree to the **Developer Certificate of
Origin (DCO)** for each commit you make. The DCO is a simple statement that you,
as a contributor, have the legal right to make the contribution.

See the [DCO](https://developercertificate.org) file for the full text of what you must agree to
and how it works [here](https://github.com/probot/dco#how-it-works).
To signify that you agree to the DCO for contributions, you simply add a line to each of your
git commit messages:

```
Signed-off-by: John Doe <[email protected]>
```

**Note:** you don't have to manually include this line on your commits, git does that for you as shown below:

```
$ git commit -s -m “my commit message w/signoff”
```

In most cases, git automatically adds the signoff to your commit with the use of
`-s` or `--signoff` flag to `git commit`. You must use your real name and a reachable email
address (sorry, no pseudonyms or anonymous contributions).

To ensure all your commits are signed, you may choose to add this alias to your global `.gitconfig`:

_~/.gitconfig_

```
[alias]
amend = commit -s --amend
cm = commit -s -m
commit = commit -s
```