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

📚 Update documentation best practices #3870

Merged
merged 4 commits into from
Jun 4, 2021
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
92 changes: 92 additions & 0 deletions docs/contributing-to-airbyte/updating-documentation.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,95 @@ If you're adding new files, don't forget to update `docs/SUMMARY.md`.

Once you're satisfied with your changes just follow the regular PR process.

## Documentation Best Practices
Connectors typically have the following documentation elements:

* READMEs
* Changelogs
* Github Issues & Pull Requests
* Source code comments
* How-to guides

Below are some best practices related to each of these.

### READMEs
Every module should have a README containing:
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe a section on setting up the source/destination in the UI, explaining the fields?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this kind of info goes into the docs/ documentation (public facing) so we probably shouldn't duplicate it? WDYT?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agree. If this is just GH facing then I'm fine with that.


* A brief description of the module
* development pre-requisites (like which language or binaries are required for development)
* how to install dependencies
* how to build and run the code locally & via Docker
* any other information needed for local iteration

### Changelogs
Copy link
Contributor

Choose a reason for hiding this comment

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

We should have people use this format for consistency:

Changelog

| Version | Date       | Pull Request | Subject |
| :------ | :--------  | :-----       | :------ |
| 0.2.0   | 20XX-05-XX | [PR2#](https://github.com/airbytehq/airbyte/pull/PR2#) | description |
| 0.1.0   | 20XX-04-XX | [PR#](https://github.com/airbytehq/airbyte/pull/PR#) | description |

Copy link
Member

Choose a reason for hiding this comment

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

@sherifnada Abhi and I are adding the changelog into the connector documentation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done


##### Core
Core changelogs should be updated in the `docs/project-overview/platform.md` file.

#### Connectors
Each connector should have a CHANGELOG.md section in its public facing docs in the `docs/integrations/<sources OR destinations>/<name>` at the bottom of the page. Inside, each new connector version should have a section whose title is the connector's version number. The body of this section should describe the changes added in the new version. For example:

```
| Version | Date | Pull Request | Subject |
| :------ | :-------- | :----- | :------ |
| 0.2.0 | 20XX-05-XX | [PR2#](https://github.com/airbytehq/airbyte/pull/PR2#) | Fixed bug with schema generation <br><br> Added a better description for the `password` input parameter |
| 0.1.0 | 20XX-04-XX | [PR#](https://github.com/airbytehq/airbyte/pull/PR#) | Added incremental sync |
```

### Source code comments
It's hard to pin down exactly what to do around source code comments, but there are two (very subjective) and rough guidelines:

**If something is not obvious, write it down**. Examples include:

* non-trivial class definitions should have docstrings
* magic variables should have comments explaining why those values are used (e.g: if using a page size of 10 in a connector, describe why if possible. If there is no reason, that's also fine, just mention in a comment).
* Complicated subroutines/logic which cannot be refactored should have comments explaining what they are doing and why

**If something is obvious, don't write it down** since it's probably more likely to go out of date. For example, a comment like `x = 42; // sets x to 42` is not adding any new information and is therefore better omitted.

### Issues & Pull Requests

#### Titles

**Describe outputs, not implementation**: An issue or PR title should describe the desired end result, not the implementation. The exception is child issues/subissues of an epic.
**Be specific about the domain**. Airbyte operates a monorepo, so being specific about what is being changed in the PR or issue title is important.

Some examples:
_subpar issue title_: `Remove airbyteCdk.dependsOn("unrelatedPackage")`. This describes a solution not a problem.

_good issue title_: `Building the Airbyte Python CDK should not build unrelated packages`. Describes desired end state and the intent is understandable without reading the full issue.

_subpar PR title_: `Update tests`. Which tests? What was the update?

_good PR title_: `Source MySQL: update acceptance tests to connect to SSL-enabled database`. Specific about the domain and change that was made.

**PR title conventions**
When creating a PR, follow the naming conventions depending on the change being made:

* Notable updates to Airbyte Core: "🎉<description of feature>"
* e.g: `🎉 enable configuring un-nesting in normalization`
* New connectors: “🎉 New source or destination: <name>” e.g: `🎉 New Source: Okta`
* New connector features: “🎉<Source or Destination> <name>: <feature description> E.g:
* `🎉 Destination Redshift: write JSONs as SUPER type instead of VARCHAR`
* `🎉 Source MySQL: enable logical replication`
* Bugfixes should start with the 🐛 emoji
* `🐛 Source Facebook Marketing: fix incorrect parsing of lookback window`
* Documentation improvements should start with any of the book/paper emojis: 📚 📝 etc…
* Any refactors, cleanups, etc.. that are not visible improvements to the user should not have emojis

The emojis help us identify which commits should be included in the product release notes.

#### Descriptions
**Context**: Provide enough information (or a link to enough information) in the description so team members with no context can understand what the issue or PR is trying to accomplish. This usually means you should include two things:

1. Some background information motivating the problem
2. A description of the problem itself
3. Good places to start reading and file changes that can be skipped
Some examples:

_insufficient context_: `Create an OpenAPI to JSON schema generator`. Unclear what the value or problem being solved here is.

_good context_:
```
When creating or updating connectors, we spend a lot of time manually transcribing JSON Schema files based on OpenAPI docs. This is ncessary because OpenAPI and JSON schema are very similar but not perfectly compatible. This process is automatable. Therefore we should create a program which converts from OpenAPI to JSONSchema format.
```