Skip to content
Open
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
132 changes: 119 additions & 13 deletions docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -117,31 +117,137 @@ fetch master and create these branches for you:

### 4. Commit your changes

Commit your change. The commit command must include a specific message format:
Commit your changes using the Conventional Commits format. All commits must follow the format described in the [Conventional Commits](#conventional-commits) section below and include proper git trailers for issue tracking.

**Basic commit format:**
```bash
git commit -m "<component>: <change_message> #<issue number>"
git commit -m "<type>(<scope>): <description>" --trailer "Closes: #<issue_number>"
```

Valid component names are listed in the [__label
list__](https://github.com/rucio/rucio/labels) and are usually specified on the
issue of the change.
**Example:**
```bash
git commit -m "feat(Core): Add new rule evaluation engine" --trailer "Closes: #1234"
```

Add additional explanations to the body of the commit, such as motivation for
certain decisions and background information. [Here are some general rules.](https://cbea.ms/git-commit/).

If you add a [__github-recognised
keyword__](https://help.github.com/articles/closing-issues-using-keywords/) then
the associated issue can be closed automatically once the pull request is
merged, e.g.:
Using multiple commits is allowed as long as they achieve an independent,
well-defined, change and are well-described. Otherwise multiple commits should
be squashed.

### **Conventional Commits**
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be one header level lower (####) since it's within the ### 4. Commit your changes step

Copy link
Contributor

Choose a reason for hiding this comment

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

(also I don't think it should be bold, since there's no other bold headers in this doc)

Copy link
Member Author

Choose a reason for hiding this comment

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

image image

You're technically right. it should be #### since it's nested. However, when I first tried that, it felt a bit off because it didn’t make “conventional commit” stand out as a distinct topic, just regular text. What are your thoughts?


Rucio enforces the [Conventional Commits](https://www.conventionalcommits.org/) specification to ensure consistent and meaningful commit messages across the project. This is enforced through [commitlint](https://commitlint.js.org/) during CI checks and can be enabled locally via pre-commit hooks.

**Commit Message Format:**
```
<type>(<scope>): <description>

[optional body]

[optional footer(s)]
```

**Rules:**

1. **Type**: Must be one of the following allowed types:
- `feat`: New feature
- `fix`: Bug fix
- `perf`: Code change that improves performance
- `docs`: Documentation only changes
- `style`: Changes that do not affect the meaning of the code
- `refactor`: Code change that neither fixes a bug nor adds a feature
- `test`: Adding missing tests or correcting existing tests
- `build`: Changes that affect the build system or external dependencies
- `ci`: Changes to CI configuration files and scripts
- `chore`: Other changes that don't modify src or test files
- `revert`: Reverts a previous commit

2. **Scope**: Must be one of the predefined Rucio components (PascalCase). The available scopes are:
- `Auth`: Authentication & Authorisation
- `Clients`: Client libraries and tools
- `Consistency`: Consistency checks
- `CI`: Continous Integration
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not a separate component, it's under Testing (https://rucio.github.io/documentation/component_leads/)

Copy link
Member Author

Choose a reason for hiding this comment

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

However, we frequently refactor our CI tooling without modifying any tests. Updates to GitHub Actions, pre-commit hooks, or CI jobs don’t affect the tests directly, so I thought it would make sense to have a separate component dedicated to changes in our GitHub CI infrastructure.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree and I brought this up to the component leads last year, but the conclusion was that we would consider CI under Testing as the component leads would likely be the same for both components anyway. So I'm not sure overall, but I think if we want to maintain the 1:1 mapping between these labels and the components, we should stick to just Testing for now and then we can consider adding a CI component and adding a label for it later on

- `Core`: Core & Internals
- `Database`: Database-related changes
- `DatasetDeletion`: Dataset deletion functionality
- `Deletion`: File deletion functionality
- `DIRAC`: DIRAC integration
- `Docker`: Docker related functionality
- `Documentation`: Documentation updates
- `Kubernetes`: Kubernetes deployment related functionality
Copy link
Contributor

Choose a reason for hiding this comment

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

Docker and Kubernetes are part of a single component https://rucio.github.io/documentation/component_leads/ - I would also be in favour of maybe just calling it Containerization so that we don't have to use tool-specific names

- `Lifetime`: Life time model processing
- `Messaging`: Messaging system
- `Metadata`: Metadata workflows
- `Monitoring`: Monitoring and observability
- `MultiVO`: Multi-VO functionality
- `Opendata`: Open data functionality
Copy link
Contributor

Choose a reason for hiding this comment

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

In theory this is not a defined component at the moment https://rucio.github.io/documentation/component_leads/, but I think it makes sense that it would be a separate component. @bari12 what do you think?

Copy link
Member Author

Choose a reason for hiding this comment

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

There’s already a GitHub label for Opendata, and since @lobis is actively working on it with commits referencing this component, I’ve added it as a component for the Conventional Commit format.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also agree this should be a distinct component.

A while back we agreed that the correct name to use would be Open Data or OpenData instead of Opendata, which I used when I first began working on the feature.

I have updated the label name to reflect this (to Open Data) and I guess the proper term in the commit messages should be OpenData.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
- `Opendata`: Open data functionality
- `OpenData`: Open data functionality

I think this would be more consistent with CERN's usage of the term as discussed in the past.

Copy link
Member

Choose a reason for hiding this comment

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

Technically, we would have to decide to add this as a component in Rucio first, which is probably a good idea :-)

Copy link
Member Author

Choose a reason for hiding this comment

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

I am wondering, how would @lobis rebase his OpenData commits with other components? for the conventional commit CI. The CI would end up failing for him.

- `Policies`: Policy management
- `Probes`: Probes & Alarms
- `Protocols`: Upload, Download, Deletion protocols
- `Rebalancing`: Data rebalancing
- `Recovery`: Data recovery
- `Replicas`: Replica workflows
- `REST`: REST & API
Copy link
Contributor

Choose a reason for hiding this comment

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

I would vote for API as component name here, rather than REST

- `Rules`: Replication rules and rule daemons
- `Subscriptions`: Subscription daemon
- `Testing`: Regression and Unit tests
- `Traces`: Monitoring & Traces
Copy link
Contributor

Choose a reason for hiding this comment

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

Monitoring and traces are part of a single component https://rucio.github.io/documentation/component_leads/

Copy link
Member Author

Choose a reason for hiding this comment

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

Should we call the component telemetry or observability, since Monitoring-Traces feels a bit too long for commit component naming?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think "monitoring" can probably be considered a superset of "traces" so monitoring on its own should be fine, but i like "observability" too. "Telemetry" has a negative connotation imo and it does not reflect what's being done here, so I would avoid using it

Copy link
Contributor

Choose a reason for hiding this comment

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

(well, it reflects what's being done here, but we're not doing the kind of telemetry that is associated with negative connotations)

- `Transfers`: Transfer daemons
- `WebUI`: Web user interface

**Note**: Any changes to this list should also be applied to the [GitHub labels](https://github.com/rucio/rucio/issues/labels) and the [commitlint config](https://github.com/rucio/rucio/blob/master/commitlint.config.js)

3. **Description**:
- Should not end with a period
- Should be concise but descriptive
- Use imperative mood ("add feature" not "added feature")

4. **Line Length**: Header must not exceed 100 characters to prevent truncation in GitHub UI

**Examples:**
```bash
<component>: <change_message> Fix #<issue number>
feat(Core): Add new rule evaluation engine
fix(DatasetDeletion): Resolve deletion timeout issues
docs(Documentation): Update API documentation
chore(CI): Update Node.js version to 24
```

Using multiple commits is allowed as long as they achieve an independent,
well-defined, change and are well-described. Otherwise multiple commits should
be squashed.
### **Git Trailers**
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should be one header level lower (####) since it's within the ### 4. Commit your changes step

Copy link
Contributor

Choose a reason for hiding this comment

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

(also I don't think it should be bold, since there's no other bold headers in this doc)


All commits must reference a GitHub issue using git trailers. This ensures proper traceability and automatic issue closure.

**Supported Trailers:**
- `Closes: #<issue_number>` - Automatically closes the issue when PR is merged
- `Issue: #<issue_number>` - References an issue without closing it

**Adding Git Trailers:**

**Method 1: Using git commit command**
```bash
git commit -m "feat(Core): Add new rule evaluation engine" --trailer "Closes: #1234"
```

**Method 2: Adding to commit body**
```bash
git commit -m "feat(Core): Add new rule evaluation engine

This commit introduces a new rule evaluation engine that improves
performance and adds support for complex rule conditions.

Closes: #1234"
```

**Complete Example:**
```bash
feat(Core): Add new rule evaluation engine

This commit introduces a new rule evaluation engine that improves
performance and adds support for complex rule conditions.

Closes: #1234
```

### 5. Push changes and create a Pull Request

Expand Down