-
Notifications
You must be signed in to change notification settings - Fork 82
docs: add AI-assisted contribution policy #606
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,68 @@ | ||||||||||
| # SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, EONERC, RWTH Aachen University | ||||||||||
| # SPDX-License-Identifier: MPL-2.0 | ||||||||||
|
|
||||||||||
| name: Compliance checks | ||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing SPDX license header The new workflow file lacks the required SPDX-License-Identifier and copyright header using the appropriate comment syntax for YAML. Absence of SPDX metadata violates the project's licensing compliance process and may cause downstream license scanning failures. Suggested fix: Add a YAML comment header at the top of the file with the SPDX lines, e.g.: SPDX-FileCopyrightText: 2025 Leonardo Carreras leonardo.carreras@eonerc.rwth-aachen.deSPDX-License-Identifier: MPL-2.0Place these lines before any other content (or after a shebang if present). Checked against the source: YAML file begins without the required SPDX license/copyright header stage: process-compliance
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Missing SPDX header in new workflow file Documentation states that all new files must include an SPDX license header. The newly added GitHub Actions workflow lacks the required SPDX-FileCopyrightText and SPDX-License-Identifier comments, violating the project’s licensing documentation policy. Suggested fix: Add an SPDX header as a YAML comment at the top of the file, e.g., SPDX-FileCopyrightText: 2025 Leonardo CarrerasSPDX-License-Identifier: MPL-2.0
Suggested change
Checked against the source: new workflow file has no SPDX-FileCopyrightText or SPDX-License-Identifier comments anywhere stage: domain-modeling |
||||||||||
|
|
||||||||||
| on: | ||||||||||
| pull_request: | ||||||||||
| branches: [master, main] | ||||||||||
|
|
||||||||||
| jobs: | ||||||||||
| no-merge-commits: | ||||||||||
| name: No merge commits | ||||||||||
| runs-on: ubuntu-latest | ||||||||||
| steps: | ||||||||||
| - uses: actions/checkout@v4 | ||||||||||
| with: | ||||||||||
| fetch-depth: 0 | ||||||||||
|
|
||||||||||
| - name: Fail if branch contains merge commits | ||||||||||
| run: | | ||||||||||
| BASE=${{ github.event.pull_request.base.sha }} | ||||||||||
| HEAD=${{ github.event.pull_request.head.sha }} | ||||||||||
| MERGES=$(git log --merges "$BASE".."$HEAD" --oneline) | ||||||||||
| if [ -n "$MERGES" ]; then | ||||||||||
| echo "Error: merge commits found in this branch:" | ||||||||||
| echo "$MERGES" | ||||||||||
| echo "" | ||||||||||
| echo "Please rebase your branch instead of merging the target branch:" | ||||||||||
| echo "" | ||||||||||
| echo " git fetch upstream" | ||||||||||
| echo " git rebase upstream/master" | ||||||||||
| echo " git push --force-with-lease" | ||||||||||
| exit 1 | ||||||||||
| fi | ||||||||||
|
|
||||||||||
| clean-notebook-outputs: | ||||||||||
| name: Notebooks have no saved outputs | ||||||||||
| runs-on: ubuntu-latest | ||||||||||
| steps: | ||||||||||
| - uses: actions/checkout@v4 | ||||||||||
| with: | ||||||||||
| fetch-depth: 0 | ||||||||||
|
|
||||||||||
| - uses: actions/setup-python@v5 | ||||||||||
| with: | ||||||||||
| python-version: '3.x' | ||||||||||
|
|
||||||||||
| - name: Fail if a notebook touched by this PR has saved outputs | ||||||||||
| run: | | ||||||||||
| BASE=${{ github.event.pull_request.base.sha }} | ||||||||||
| HEAD=${{ github.event.pull_request.head.sha }} | ||||||||||
| NOTEBOOKS=$(git diff --name-only --diff-filter=d "$BASE" "$HEAD" -- '*.ipynb') | ||||||||||
| if [ -z "$NOTEBOOKS" ]; then | ||||||||||
Check warningCode scanning / SonarCloud Python package manager scripts should not be executed during installation Medium
Omitting "--only-binary :all:" can lead to the execution of setup scripts. Make sure it is safe here. See more on SonarQube Cloud
Check warningCode scanning / SonarCloud Python dependencies should be locked to verified versions Medium
Using dependencies without locking resolved versions is security-sensitive. See more on SonarQube Cloud
|
||||||||||
|
|
||||||||||
| exit 0 | ||||||||||
| fi | ||||||||||
| pip install nbconvert | ||||||||||
|
Check warning on line 56 in .github/workflows/compliance.yaml
|
||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pin nbconvert version in CI to avoid breakage The CI step installs nbconvert without a pinned version ('pip install nbconvert'). This risks CI failures if nbconvert releases a breaking change or removes a previously available version. Pinning the version ensures reproducibility and aligns with best practices for dependency management in CI. Suggested fix: Pin nbconvert to a known-good version in the CI workflow, e.g., 'pip install nbconvert==7.16.6'.
Suggested change
Checked against the source: the CI step runs pip install nbconvert without pinning a version stage: build-deps
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pin nbconvert version to avoid CI breakage The workflow installs nbconvert without a pinned version, which risks CI failures when a new nbconvert release introduces breaking changes. This violates DPsim's maintainability and reproducibility conventions. Suggested fix: Pin nbconvert to a known-good version (e.g., 'nbconvert==7.16.4') to ensure the notebook stripping step remains stable across CI runs. Checked against the source: the workflow installs nbconvert with pip install nbconvert and no version constraint stage: numerics |
||||||||||
| python -m nbconvert --ClearOutputPreprocessor.enabled=True --inplace $NOTEBOOKS | ||||||||||
| if ! git diff --quiet -- $NOTEBOOKS; then | ||||||||||
| echo "Error: the following notebooks have saved outputs committed:" | ||||||||||
| git diff --name-only -- $NOTEBOOKS | ||||||||||
| echo "" | ||||||||||
| echo "Strip outputs before committing:" | ||||||||||
| echo "" | ||||||||||
| echo " jupyter nbconvert --ClearOutputPreprocessor.enabled=True --inplace <notebook.ipynb>" | ||||||||||
| echo "" | ||||||||||
| echo "Or install pre-commit hooks (see CONTRIBUTING.md) to do this automatically." | ||||||||||
| exit 1 | ||||||||||
| fi | ||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -48,3 +48,29 @@ repos: | |||||||||||||||||||||||||||||||||||||||||||||
| hooks: | ||||||||||||||||||||||||||||||||||||||||||||||
| - id: markdownlint | ||||||||||||||||||||||||||||||||||||||||||||||
| args: [-r, "~MD013,~MD033,~MD024"] | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - repo: local | ||||||||||||||||||||||||||||||||||||||||||||||
| hooks: | ||||||||||||||||||||||||||||||||||||||||||||||
| - id: clear-notebook-outputs | ||||||||||||||||||||||||||||||||||||||||||||||
| name: clear notebook outputs | ||||||||||||||||||||||||||||||||||||||||||||||
| language: python | ||||||||||||||||||||||||||||||||||||||||||||||
| additional_dependencies: ["nbconvert"] | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pin nbconvert version in pre-commit hook to avoid breakage The pre-commit hook declares nbconvert as an additional dependency without a pinned version ('additional_dependencies: ["nbconvert"]'). This risks breakage if nbconvert releases a breaking change. Pinning the version ensures the hook remains stable across environments. Suggested fix: Pin nbconvert to the same version used in CI, e.g., 'additional_dependencies: ["nbconvert==7.16.6"]'. Checked against the source: additional_dependencies lists nbconvert without a version pin stage: build-deps |
||||||||||||||||||||||||||||||||||||||||||||||
| entry: python -m nbconvert --ClearOutputPreprocessor.enabled=True --inplace | ||||||||||||||||||||||||||||||||||||||||||||||
| files: \.ipynb$ | ||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||
| - id: no-merge-commits | ||||||||||||||||||||||||||||||||||||||||||||||
| name: no merge commits (use rebase) | ||||||||||||||||||||||||||||||||||||||||||||||
| language: system | ||||||||||||||||||||||||||||||||||||||||||||||
| entry: >- | ||||||||||||||||||||||||||||||||||||||||||||||
| bash -c '[ ! -f .git/MERGE_HEAD ] || { | ||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Incorrect merge‑commit detection in pre‑commit hook The Suggested fix: Replace the check with a command that inspects the branch history for merge commits, e.g.,
Suggested change
Checked against the source: the no-merge-commits hook checks only .git/MERGE_HEAD and therefore misses existing merge commits in the branch history stage: cpp-design
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no-merge-commits hook only checks MERGE_HEAD The local pre-commit hook "no-merge-commits" determines a violation by testing the existence of .git/MERGE_HEAD. This file is present only while a merge is in progress, not when the branch already contains merge commits in its history. Consequently, a developer can commit a branch that includes merge commits and the hook will pass, relying solely on the CI job to catch the issue. This defeats the purpose of the pre‑commit guard and allows merge commits to slip into the repository if CI is bypassed. Suggested fix: Replace the MERGE_HEAD test with a git log based detection of merge commits in the current branch, analogous to the CI job. For example, run
Suggested change
Checked against the source: the local hook determines failure solely from .git/MERGE_HEAD, which is present only during an ongoing merge stage: io-robustness
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve no-merge-commits hook robustness The no-merge-commits pre-commit hook checks for the presence of .git/MERGE_HEAD to detect merge commits. This file is not guaranteed to exist in all Git environments (e.g., shallow clones or CI runners with fetch-depth=1), which can cause the hook to silently pass even when merge commits are present. This undermines the linear-history policy. Suggested fix: Replace the file-based check with a Git command that directly inspects the commit history, e.g. use 'git merge-base --is-ancestor' or 'git log --merges' to detect merge commits regardless of .git/MERGE_HEAD presence. Update the entry to a shell script that always uses 'git' commands and exits non-zero when merge commits are found. Checked against the source: the hook only tests for .git/MERGE_HEAD, which detects an in-progress merge rather than merge commits already present in branch history stage: realtime-resources |
||||||||||||||||||||||||||||||||||||||||||||||
| echo ""; | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "Error: merge commits are not allowed in DPsim."; | ||||||||||||||||||||||||||||||||||||||||||||||
| echo "Please rebase your branch onto the target branch instead:"; | ||||||||||||||||||||||||||||||||||||||||||||||
| echo ""; | ||||||||||||||||||||||||||||||||||||||||||||||
| echo " git rebase upstream/master"; | ||||||||||||||||||||||||||||||||||||||||||||||
| echo " git push --force-with-lease"; | ||||||||||||||||||||||||||||||||||||||||||||||
| echo ""; | ||||||||||||||||||||||||||||||||||||||||||||||
| exit 1; }' | ||||||||||||||||||||||||||||||||||||||||||||||
| stages: [pre-commit] | ||||||||||||||||||||||||||||||||||||||||||||||
| always_run: true | ||||||||||||||||||||||||||||||||||||||||||||||
| pass_filenames: false | ||||||||||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| <!-- | ||
| SPDX-FileCopyrightText: 2026 Institute for Automation of Complex Power Systems, EONERC, RWTH Aachen University | ||
| SPDX-License-Identifier: MPL-2.0 | ||
| --> | ||
|
|
||
| # Contributing to DPsim | ||
|
|
||
| Thanks for your interest in contributing! Below are the essentials to get a pull request merged. For full detail see the [Development Guidelines](https://dpsim.fein-aachen.org/docs/development/guidelines/). | ||
|
|
||
| ## Quick start | ||
|
|
||
| 1. Fork the repository and clone your fork. | ||
| 1. Run `pre-commit install` to activate automated checks (formatting, linear history guard, notebook output stripping). | ||
| 1. Create a branch with a descriptive prefix (`feature/`, `fix/`, `docs/`). | ||
| 1. Make your changes and commit using the conventional commit style (`feat:`, `fix:`, `docs:`, ...) with a sign-off: | ||
|
|
||
| ```shell | ||
| git commit -s -m "fix: correct node voltage initialization in DiakopticsSolver" | ||
| ``` | ||
|
|
||
| 1. Keep your branch up to date by rebasing; do not merge the target branch into your branch: | ||
|
|
||
| ```shell | ||
| git fetch upstream | ||
| git rebase upstream/master | ||
| git push --force-with-lease | ||
| ``` | ||
|
|
||
| 1. Open a pull request against `sogno-platform/dpsim:master`. | ||
|
|
||
| ## Key requirements | ||
|
|
||
| - **Linear history**: merge commits in a feature branch are blocked by CI; rebase instead. | ||
| - **Sign-off**: every commit must include `Signed-off-by` via `git commit -s` ([DCO](https://developercertificate.org/)). | ||
| - **Formatting**: clang-format 17 for C++, black for Python, markdownlint for Markdown; all enforced by pre-commit. | ||
| - **SPDX headers**: new files must include a license header (see [guidelines](https://dpsim.fein-aachen.org/docs/development/guidelines/)). | ||
| - **No `std::cout` in C++**: use `SPDLOG_LOGGER_*` macros. | ||
| - **No saved notebook outputs**: strip outputs from `.ipynb` files before committing; pre-commit and CI both enforce this. | ||
| - **Contributions in forks only**: do not push feature branches to the main repository. | ||
|
|
||
| ## AI-assisted contributions | ||
|
|
||
| DPsim follows the Linux Foundation [policy guidance on generative AI tools](https://www.linuxfoundation.org/legal/generative-ai), which permits AI-generated contributions and sets out what to check before making one. Two of its requirements bear directly on this project: the tool's terms must not restrict its output in ways that conflict with the [MPL-2.0](https://mozilla.org/MPL/2.0/), and where the output carries pre-existing third-party material you must have permission to contribute it and must supply the corresponding notice and attribution alongside your change. That guidance invites projects to add their own; the rest of this section is ours. | ||
|
|
||
| Using an assistant changes nothing about who is accountable for the result, and none of this is peculiar to AI. Code you adapted from a forum answer, a blog post or another project has always carried the same obligations, and an assistant is not a special case. You are the author of everything you submit: read it, build it, run the tests, and be ready to defend it in review exactly as if you had typed it. Do not open a pull request containing code you cannot explain. Your `Signed-off-by` line is a statement under the [DCO](https://developercertificate.org/) that you have the right to submit the work, so sign it only once the checks above hold. | ||
|
|
||
| Say so in the pull request description when a change is largely generated, so reviewers know where to look harder. Do not add AI co-author trailers to commit messages; the sign-off already identifies the author. Numerical results, validation notebooks and physical reasoning need the same scrutiny as code, and plausible-looking output is not evidence a model is correct. | ||
|
|
||
| Do not paste unpublished research, confidential data or third-party code you do not own into an external service. | ||
|
|
||
| ## License | ||
|
|
||
| By contributing you agree your work is released under the [Mozilla Public License 2.0](https://mozilla.org/MPL/2.0/). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add SPDX license header to compliance.yaml
severity: high · confidence: 96%compliance.yaml is a new GitHub Actions workflow file and must include an SPDX license header in the file's own comment syntax. YAML files use # for comments, so the header should be the first line of the file.
Suggested fix: Add the following SPDX header as the first line of compliance.yaml:
SPDX-FileCopyrightText: 2025 Contributors to DPsim
SPDX-License-Identifier: MPL-2.0
Immediately after the shebang line if present, or as the first line otherwise.
Checked against the source: new workflow starts with
name: Compliance checksand contains no SPDX comment headerstage: process-compliance