Skip to content

Commit

Permalink
Add contribution guide
Browse files Browse the repository at this point in the history
  • Loading branch information
JeremyMcCormick committed Dec 10, 2024
1 parent 5ecf80e commit ec2a12e
Showing 1 changed file with 97 additions and 0 deletions.
97 changes: 97 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
Contributing to SDM Schemas
===========================

Creating a PR
-------------

All changes to the repository must be made by [opening a pull request](https://github.com/lsst/sdm_schemas/compare), which will require one approving reviewer. Any branches used for PRs which contain non-trivial changes should follow the [standard DM naming convention for ticket branches](https://developer.lsst.io/work/flow.html#ticket-branches), or the PR will be rejected and closed.

The PR title should be formatted like: `[TICKET]: [TITLE]` where `TICKET` is the Jira ticket number and `TITLE` is a short description.

For example, this is a valid PR title, with a dummy ticket number:

`DM-12345: Add ra and dec columns to test schema`.

Including the name of the schema in the PR title is also helpful for the reviewer and provenance, e.g., `test` in this case.

The description body of the PR should contain clear information on what is being changed in the schema, though this may be brief.
A set of bullet points describing each change or a few sentences will usually suffice.
The Jira ticket page can be used as a source of extended information and discussion on the issue being resolved.

An appropriate reviewer should be assigned, based on which schema is being changed or added. If the author does not know who should review their PR, [JeremyMcCormick](https://github.com/JeremyMcCormick) can be assigned as a default, and an appropriate reviewer will be found.

The author should [resolve the review](https://developer.lsst.io/work/flow.html#resolving-a-review) if changes are requested. Once the PR is approved, it may then be merged by the author.

Pull Request Checks
-------------------

SDM Schemas pull requests have an extensive set of checks that will run automatically.
Aside from special cases, a pull request will have to pass all of these checks before it can be merged.

These checks will:

- [Build TAP_SCHEMA and DataLink resources](.github/workflows/build.yaml)
- [Compare Schemas for Changes](.github/workflows/compare.yaml) - Schemas that are in the [deployed list](./yml/deployed-schemas.txt) will fail this check.
- [Build the Schema Browser website](./yml/docs.yaml)
- [Check that `main` is not merged into the branch](./yml/rebase_checker.yaml)
- [Test that the schemas can be used to create database](./yml/test_databases.yml)
- [Vaildate the schemas using Felis](./yml/validate.yaml)
- [Lint YAML files](./yml/yamllint.yaml)

The checks will rerun anytime the PR branch is updated.

Rebasing
--------

If a pull request shows the message "This branch is out-of-date with the base branch", then the "Update with rebase" button should be selected and pressed on the right to update it.
The "Update branch" selection should not be used, as it will perform a merge, which is undesirable behavior that will cause the rebase check to fail.

Rebasing can also be accomplished using a command like `git rebase -i main` locally after updating `main` and then force pushing the rebased branch using `git push -f`.

Schema Versioning Overview
--------------------------

Individual schemas may have their own internal [version](https://felis.lsst.io/user-guide/model.html#schema-version) for tracking changes, which is defined at the top level of the file, as in [apdb.yaml](yml/apdb.yaml).
(This is distinct from tags or versions of sdm_schemas itself, and not all schemas may be using these internal versions.)
This version may need to be updated when making changes.

Usage of semantic versioning is recommended, which defines a version like `MAJOR.MINOR.PATCH`, such as `1.2.3`.

Semantic versioning typically follows the following generic guidelines:

- `MAJOR`: Breaking changes that are not backward compatible.
- `MINOR`: Backward-compatible changes.
- `PATCH`: Backward-compatible changes, typically those that do not change the database schema.

These rules would typically not be followed strictly for schema changes. For instance, some operations which are technically backward compatible, such as adding a table, should likely result in a `MAJOR` version increment rather than `MINOR`, as they constitute an important and significant update which would usually require major updates to client queries and APIs.

Versioning Guidelines
---------------------

Below are some general guidelines that can be used for incrementing the schema version based on what changes are being made:

| Operation | Increment | Notes |
|--------|-----------|-------|
| Add table | MAJOR | Even if backward compatible, new tables often signal major functionality changes, usually requiring clients to update their code or queries. |
| Remove table | MAJOR | Removing a table may break existing queries and client interfaces, making it incompatible with previous versions. |
| Add column | MAJOR | Adding a column may require clients to adapt to the new data structure (e.g., parsing new fields), treated as a major change for consistency. |
| Remove column | MAJOR | Removing a column breaks queries and code expecting that column, thus not backward compatible. |
| Change column type| MAJOR | Changing a column’s data type can break clients expecting a different type, forcing a major version increment. |
| Add index | PATCH | Indexes improve performance without changing the schema’s interface, making this a safe, backward-compatible improvement. |
| Add constraint | MAJOR | New constraints can invalidate previously acceptable data, breaking clients that rely on the old lenient rules. |
| Change constraint | MAJOR | Altered constraints can break clients relying on the previous constraint behavior, making this a non-backward-compatible change. |
| Remove constraint | MINOR | Removing a constraint generally relaxes restrictions. It’s unlikely to break clients, but it changes behavior enough to warrant more than a patch. |
| Change metadata | PATCH | Adjusting metadata that does not affect the schema’s functional behavior or data shape is a minor adjustment that remains backward compatible. |

The above are only suggestions.
The [specific schema documentation](#specific-schema-documentation) should be consulted for more specific rules and guidelines on version incrementing.

Database Migration
------------------

Database migrations are currently performed with schema-specific tooling in external repositories, so documentation for that particular schema should be consulted on how to perform them and how this may impact the pull request process.

Specific Schema Documentation
-----------------------------

- [Alert Production Database (APDB)](docs/APDB.md)

0 comments on commit ec2a12e

Please sign in to comment.