🚧 Update commit message format
This commit is contained in:
Jonas Bengtsson
@@ -18,6 +18,7 @@ See `functional specification.md` for the full specification.
|
||||
- Security validation with forbidden patterns (abort by default)
|
||||
- Pluggable author resolvers with priority chain and confidence levels
|
||||
- Per-file attribution recorded in the commit message
|
||||
- One-line-per-file commit entries in `git diff --name-status` style (letter, path, author, last-modified time), with a configurable separator (`commit.file_separator`, default `;`)
|
||||
- Structured commit trailers (`Tracker-Run`, `Tracker-Host`, ...)
|
||||
- Repository-level lock with stale-lock detection (stored outside the repo)
|
||||
- Rotating file logs plus console logging
|
||||
@@ -69,7 +70,7 @@ scope is defined by Section 44, *Definition of Done – Initial Release*.
|
||||
- [x] Configurable author resolver chain with confidence levels and `UNKNOWN` fallback; sidecar metadata resolver implemented (FR-006, FR-007, Sections 9 – 11)
|
||||
- [x] Per-file attribution in the commit message (FR-008, FR-012)
|
||||
- [x] Automatic commit generation (FR-009)
|
||||
- [x] Structured commit message: summary counts, per-file details, `Tracker-*` trailers (FR-010, FR-011, Sections 15/16/36)
|
||||
- [x] Structured commit message: summary counts, one-line-per-file entries with change letter / path / author / local-time mtime, `Tracker-*` trailers (FR-010, FR-011, Sections 15/16/36)
|
||||
- [x] No empty commits (FR-014)
|
||||
- [x] Rotating application logs, console + file (FR-015, Sections 22 – 23)
|
||||
- [x] Cross-platform code base: `pathlib`, argument arrays, UTF-8/NUL-safe Git parsing (FR-016, FR-029)
|
||||
|
||||
@@ -36,6 +36,12 @@ author_resolution:
|
||||
minimum_confidence: probable
|
||||
fallback: unknown
|
||||
|
||||
commit:
|
||||
# Separator between the parts of a one-line file entry in commit
|
||||
# messages (Section 15). Must not occur inside paths, author
|
||||
# identifiers or timestamps.
|
||||
file_separator: ";"
|
||||
|
||||
logging:
|
||||
level: INFO
|
||||
console: true
|
||||
|
||||
+31
-30
@@ -526,10 +526,21 @@ If the staged change set does not match the expected change set, the commit shal
|
||||
|
||||
Commit messages shall be human-readable while retaining a predictable structure that can later be parsed automatically.
|
||||
|
||||
Each changed file shall occupy exactly one line, in the style of `git diff --name-status`. A single-letter change type is followed by the file path, the last modifying user and the filesystem last-modified time, with the parts joined by a configurable separator (see Section 26, `commit.file_separator`; default `;`).
|
||||
|
||||
Line grammar:
|
||||
|
||||
```text
|
||||
A <sep> <path> <sep> <author> <sep> <modified-at> added
|
||||
M <sep> <path> <sep> <author> <sep> <modified-at> modified
|
||||
R <sep> <old-path> -> <new-path> renamed (bare)
|
||||
D <sep> <path> deleted (bare)
|
||||
```
|
||||
|
||||
Recommended format:
|
||||
|
||||
```text
|
||||
SCADA automatic change capture - 2026-09-11 18:00:03
|
||||
SCADA automatic change capture - 2026-09-11 18:00:03+02:00
|
||||
|
||||
Summary:
|
||||
Added: 2
|
||||
@@ -537,31 +548,19 @@ Summary:
|
||||
Deleted: 1
|
||||
Renamed: 0
|
||||
|
||||
Added:
|
||||
Stations/PS101/pump2.xml
|
||||
Last modified by: DOMAIN\jsmith
|
||||
Modified at: 2026-09-11 15:41:22
|
||||
|
||||
Stations/PS104/settings.xml
|
||||
Last modified by: DOMAIN\adoe
|
||||
Modified at: 2026-09-11 16:02:14
|
||||
|
||||
Modified:
|
||||
Stations/PS101/project.xml
|
||||
Last modified by: DOMAIN\jsmith
|
||||
Modified at: 2026-09-11 15:42:09
|
||||
|
||||
Stations/PS103/alarm.xml
|
||||
Last modified by: DOMAIN\adoe
|
||||
Modified at: 2026-09-11 16:13:54
|
||||
|
||||
System/global.xml
|
||||
Last modified by: UNKNOWN
|
||||
|
||||
Deleted:
|
||||
Stations/PS027/old-config.xml
|
||||
Changes:
|
||||
A ; Stations/PS101/pump2.xml ; DOMAIN\jsmith ; 2026-09-11 15:41:22+02:00
|
||||
A ; Stations/PS104/settings.xml ; DOMAIN\adoe ; 2026-09-11 16:02:14+02:00
|
||||
M ; Stations/PS101/project.xml ; DOMAIN\jsmith ; 2026-09-11 15:42:09+02:00
|
||||
M ; Stations/PS103/alarm.xml ; DOMAIN\adoe ; 2026-09-11 16:13:54+02:00
|
||||
M ; System/global.xml ; UNKNOWN ; 2026-09-11 16:20:01+02:00
|
||||
D ; Stations/PS027/old-config.xml
|
||||
```
|
||||
|
||||
Deleted files are listed without author or time information (FR-013). Renames use git's `old -> new` arrow notation and stay bare: a pure rename preserves the original modification time, so a timestamp would be misleading. The time column is the filesystem last-modified time (Section 12: `file_timestamp`) rendered in local time with an explicit UTC offset, matching the log timestamp style (Section 22).
|
||||
|
||||
The one-line format keeps the message proportional to the number of changed files, which matters for large collections.
|
||||
|
||||
---
|
||||
|
||||
# 16. Commit Subject
|
||||
@@ -609,12 +608,9 @@ One execution may contain changes made by multiple individuals.
|
||||
Example:
|
||||
|
||||
```text
|
||||
Modified:
|
||||
plant1.xml
|
||||
Last modified by: DOMAIN\jsmith
|
||||
|
||||
plant2.xml
|
||||
Last modified by: DOMAIN\adoe
|
||||
Changes:
|
||||
M ; plant1.xml ; DOMAIN\jsmith ; 2026-09-11 15:42:09+02:00
|
||||
M ; plant2.xml ; DOMAIN\adoe ; 2026-09-11 16:02:14+02:00
|
||||
```
|
||||
|
||||
The application shall therefore store attribution per file rather than per commit.
|
||||
@@ -838,6 +834,11 @@ author_resolution:
|
||||
minimum_confidence: probable
|
||||
fallback: unknown
|
||||
|
||||
commit:
|
||||
# Separator between the parts of a one-line file entry (Section 15).
|
||||
# Must not occur inside paths, author identifiers or timestamps.
|
||||
file_separator: ";"
|
||||
|
||||
logging:
|
||||
level: INFO
|
||||
console: true
|
||||
|
||||
@@ -171,7 +171,12 @@ class ApplicationController:
|
||||
"Author resolved: %s -> %s", record.path, result.identifier
|
||||
)
|
||||
|
||||
message = build_commit_message(kept, run, app_version=_APP_VERSION)
|
||||
message = build_commit_message(
|
||||
kept,
|
||||
run,
|
||||
app_version=_APP_VERSION,
|
||||
file_separator=self.config.commit.file_separator,
|
||||
)
|
||||
|
||||
if dry_run:
|
||||
log.info("Dry-run: repository left unmodified")
|
||||
|
||||
@@ -2,13 +2,17 @@
|
||||
|
||||
Human-readable sections first, Git trailer block last so that
|
||||
`git interpret-trailers` can extract the structured metadata.
|
||||
|
||||
Each changed file occupies exactly ONE line in `git diff --name-status`
|
||||
style: a change letter followed by path, author and last-modified time,
|
||||
joined by a configurable separator (commit.file_separator, default ';').
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import datetime as dt
|
||||
|
||||
from git_auto_tracker.changes.models import ChangeSet, TrackerRun
|
||||
from git_auto_tracker.changes.models import ChangeRecord, ChangeSet, ChangeType, TrackerRun
|
||||
|
||||
_SUMMARY_LABELS = (
|
||||
("Added:", "added"),
|
||||
@@ -17,6 +21,13 @@ _SUMMARY_LABELS = (
|
||||
("Renamed:", "renamed"),
|
||||
)
|
||||
|
||||
_GIT_LETTER = {
|
||||
ChangeType.ADDED: "A",
|
||||
ChangeType.MODIFIED: "M",
|
||||
ChangeType.DELETED: "D",
|
||||
ChangeType.RENAMED: "R",
|
||||
}
|
||||
|
||||
|
||||
def _format_ts(moment: dt.datetime | None) -> str:
|
||||
"""Render an aware datetime in local time with an explicit offset.
|
||||
@@ -30,20 +41,28 @@ def _format_ts(moment: dt.datetime | None) -> str:
|
||||
return moment.astimezone().isoformat(sep=" ", timespec="seconds")
|
||||
|
||||
|
||||
def _author_lines(record) -> list[str]: # noqa: ANN001 - ChangeRecord, avoid cycle
|
||||
lines: list[str] = []
|
||||
author = record.author
|
||||
identifier = author.identifier if author is not None else "UNKNOWN"
|
||||
lines.append(f" Last modified by: {identifier}")
|
||||
# Filesystem last-modified time (Section 12: file_timestamp); rendered
|
||||
# in local time like the log timestamps. Resolver-provided timestamps
|
||||
# (author.timestamp) stay authoritative and are shown as well.
|
||||
file_ts = _format_ts(record.file_timestamp)
|
||||
if file_ts:
|
||||
lines.append(f" Modified at: {file_ts}")
|
||||
if author is not None and author.timestamp is not None:
|
||||
lines.append(f" Changed at: {_format_ts(author.timestamp)}")
|
||||
return lines
|
||||
def _file_line(record: ChangeRecord, separator: str) -> str:
|
||||
"""One line per file, git --name-status style (Section 15).
|
||||
|
||||
Grammar:
|
||||
A <sep> <path> <sep> <author> <sep> <mtime> added
|
||||
M <sep> <path> <sep> <author> <sep> <mtime> modified
|
||||
R <sep> <old-path> -> <new-path> renamed (bare)
|
||||
D <sep> <path> deleted (bare)
|
||||
|
||||
The filesystem last-modified time (Section 12: file_timestamp) is used
|
||||
for the time column, rendered in local time like the log timestamps.
|
||||
Deleted and renamed entries stay bare: the file is gone (FR-013), and a
|
||||
pure rename preserves the original mtime so a timestamp would mislead.
|
||||
"""
|
||||
letter = _GIT_LETTER[record.change_type]
|
||||
if record.change_type is ChangeType.RENAMED:
|
||||
return f"{letter} {separator} {record.previous_path or record.path} -> {record.path}"
|
||||
if record.change_type is ChangeType.DELETED:
|
||||
return f"{letter} {separator} {record.path}"
|
||||
author = record.author.identifier if record.author is not None else "UNKNOWN"
|
||||
parts = [letter, record.path, author, _format_ts(record.file_timestamp)]
|
||||
return f" {separator} ".join(parts)
|
||||
|
||||
|
||||
def build_commit_message(
|
||||
@@ -51,6 +70,7 @@ def build_commit_message(
|
||||
run: TrackerRun,
|
||||
collected_at: dt.datetime | None = None,
|
||||
app_version: str = "1.0.0",
|
||||
file_separator: str = ";",
|
||||
) -> str:
|
||||
"""Build the complete commit message per Sections 15/16/36."""
|
||||
counts = change_set.summary_counts()
|
||||
@@ -64,22 +84,9 @@ def build_commit_message(
|
||||
lines.append(f" {label} {counts[key]}")
|
||||
lines.append("")
|
||||
|
||||
for heading, records in (
|
||||
("Added:", change_set.added),
|
||||
("Modified:", change_set.modified),
|
||||
("Renamed:", change_set.renamed),
|
||||
("Deleted:", change_set.deleted),
|
||||
):
|
||||
if not records:
|
||||
continue
|
||||
lines.append(heading)
|
||||
for record in records:
|
||||
if record.change_type.value == "RENAMED" and record.previous_path:
|
||||
lines.append(f" {record.previous_path} -> {record.path}")
|
||||
else:
|
||||
lines.append(f" {record.path}")
|
||||
if record.needs_author:
|
||||
lines.extend(_author_lines(record))
|
||||
lines.append("Changes:")
|
||||
for record in change_set.all_records():
|
||||
lines.append(f" {_file_line(record, file_separator)}")
|
||||
lines.append("")
|
||||
|
||||
# Structured trailers (Section 36)
|
||||
|
||||
@@ -90,6 +90,24 @@ class LoggingConfig:
|
||||
backup_count: int = 10
|
||||
|
||||
|
||||
@dataclass
|
||||
class CommitConfig:
|
||||
"""Commit-message rendering options (Sections 15 and 16)."""
|
||||
|
||||
#: Separator between the parts of a one-line file entry. Must not be
|
||||
#: empty; must not occur inside paths, author identifiers or timestamps.
|
||||
file_separator: str = ";"
|
||||
|
||||
@classmethod
|
||||
def from_dict(cls, data: dict[str, Any]) -> CommitConfig:
|
||||
separator = data.get("file_separator", ";")
|
||||
if not isinstance(separator, str) or not separator.strip():
|
||||
raise ConfigurationError("commit.file_separator must be a non-empty string")
|
||||
if separator in ("\n", "\r"):
|
||||
raise ConfigurationError("commit.file_separator must not be a line break")
|
||||
return cls(file_separator=separator)
|
||||
|
||||
|
||||
@dataclass
|
||||
class AppConfig:
|
||||
repository_path: Path
|
||||
@@ -101,6 +119,7 @@ class AppConfig:
|
||||
author_resolution: AuthorResolutionConfig
|
||||
logging: LoggingConfig
|
||||
timezone: str | None = None
|
||||
commit: CommitConfig = field(default_factory=CommitConfig)
|
||||
|
||||
@classmethod
|
||||
def from_dict(cls, data: dict[str, Any], config_dir: Path) -> AppConfig:
|
||||
@@ -152,6 +171,7 @@ class AppConfig:
|
||||
max_size_mb=int(logging_cfg.get("max_size_mb", 20)),
|
||||
backup_count=int(logging_cfg.get("backup_count", 10)),
|
||||
),
|
||||
commit=CommitConfig.from_dict(data.get("commit", {})),
|
||||
timezone=app_cfg.get("timezone"),
|
||||
)
|
||||
except ConfigurationError:
|
||||
|
||||
@@ -100,23 +100,36 @@ class TestRunCycle:
|
||||
)
|
||||
_run(tracker_config, tmp_path)
|
||||
body = _git_out("log", "-1", "--format=%b", cwd=git_repo)
|
||||
assert "Last modified by: DOMAIN\\jsmith" in body
|
||||
# One-line entry: letter ; path ; author ; local-time mtime
|
||||
assert " A ; config.xml ; DOMAIN\\jsmith ; " in body
|
||||
|
||||
def test_file_mtime_appears_in_commit_local_time(
|
||||
self, git_repo: Path, tracker_config: dict, tmp_path: Path
|
||||
) -> None:
|
||||
"""Section 12 file_timestamp rendered as 'Modified at' (local time)."""
|
||||
"""Section 12 file_timestamp on the one-line entry (local time)."""
|
||||
import re as re_mod
|
||||
|
||||
(git_repo / "config.xml").write_text("<c/>", encoding="utf-8")
|
||||
_run(tracker_config, tmp_path)
|
||||
body = _git_out("log", "-1", "--format=%b", cwd=git_repo)
|
||||
# Local-time format with explicit +/-HH:MM offset (Section 15)
|
||||
# A ; path ; author ; local-time mtime with +/-HH:MM offset
|
||||
assert re_mod.search(
|
||||
r"Modified at: \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}\b", body
|
||||
r"^ A ; config\.xml ; UNKNOWN ; "
|
||||
r"\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}$",
|
||||
body,
|
||||
flags=re_mod.MULTILINE,
|
||||
)
|
||||
# The timestamp block sits directly under the changed file entry
|
||||
assert " config.xml\n Last modified by: UNKNOWN\n Modified at:" in body
|
||||
|
||||
def test_custom_separator_in_real_commit(
|
||||
self, git_repo: Path, tracker_config: dict, tmp_path: Path
|
||||
) -> None:
|
||||
"""commit.file_separator flows from configuration into the commit."""
|
||||
tracker_config["commit"] = {"file_separator": "|"}
|
||||
(git_repo / "config.xml").write_text("<c/>", encoding="utf-8")
|
||||
_run(tracker_config, tmp_path)
|
||||
body = _git_out("log", "-1", "--format=%b", cwd=git_repo)
|
||||
assert " A | config.xml | UNKNOWN | " in body
|
||||
assert "; config.xml" not in body
|
||||
|
||||
def test_excluded_files_not_committed(
|
||||
self, git_repo: Path, tracker_config: dict, tmp_path: Path
|
||||
|
||||
@@ -102,6 +102,7 @@ class TestConfig:
|
||||
assert config.repository_path == Path(tracker_config["repository"]["path"])
|
||||
assert config.branch == "main"
|
||||
assert config.author_resolution.minimum_confidence == "probable"
|
||||
assert config.commit.file_separator == ";" # default
|
||||
|
||||
def test_missing_file(self, tmp_path: Path) -> None:
|
||||
with pytest.raises(ConfigurationError):
|
||||
@@ -175,16 +176,19 @@ class TestCommitMessage:
|
||||
assert lines[0].startswith("SCADA automatic change capture - ")
|
||||
assert "Summary:" in lines
|
||||
assert "Added: 1" in message
|
||||
assert "Last modified by: DOMAIN\\jsmith" in message
|
||||
assert "Last modified by: UNKNOWN" in message
|
||||
# Filesystem mtime rendered as 'Modified at' in local time with offset
|
||||
assert " Modified at: " in message
|
||||
assert "Deleted: 1" in message
|
||||
# One-line-per-file entries with the default ';' separator (Section 15)
|
||||
assert " A ; Stations/PS101/pump2.xml ; UNKNOWN ; " in message
|
||||
assert " M ; Stations/PS101/project.xml ; DOMAIN\\jsmith ; " in message
|
||||
assert " M ; System/global.xml ; UNKNOWN ; " in message
|
||||
assert " D ; Stations/PS027/old.xml" in message
|
||||
# Local-time timestamps with explicit +/-HH:MM offset
|
||||
assert re.search(
|
||||
r"Modified at: \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}\b", message
|
||||
r"; \d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}[+-]\d{2}:\d{2}\b", message
|
||||
)
|
||||
# Resolver-provided timestamp kept as a separate 'Changed at' line
|
||||
assert " Changed at: " in message
|
||||
assert "Stations/PS027/old.xml" in message
|
||||
# One 'Changes:' section; no multi-line author blocks remain
|
||||
assert "Last modified by:" not in message
|
||||
assert "Changes:" in message
|
||||
# Trailers last (Section 36)
|
||||
assert "Tracker-Version: 1.0.0" in message
|
||||
assert "Tracker-Run: 01J7K8HYV84ZKFH1J4T9R8DZQN" in message
|
||||
@@ -199,9 +203,12 @@ class TestCommitMessage:
|
||||
deleted=[ChangeRecord(path="gone.xml", change_type=ChangeType.DELETED)]
|
||||
)
|
||||
message = build_commit_message(change_set, self._run())
|
||||
idx = message.index("gone.xml")
|
||||
following = message[idx:].splitlines()
|
||||
assert not any("Last modified by" in line for line in following[1:3])
|
||||
# Deleted entries are bare: letter, separator, path - no author/time
|
||||
assert " D ; gone.xml" in message
|
||||
assert "UNKNOWN" not in message
|
||||
for line in message.splitlines():
|
||||
if "gone.xml" in line:
|
||||
assert line.count(";") == 1
|
||||
|
||||
def test_rename_rendered_with_arrow(self) -> None:
|
||||
change_set = ChangeSet(
|
||||
@@ -212,4 +219,15 @@ class TestCommitMessage:
|
||||
]
|
||||
)
|
||||
message = build_commit_message(change_set, self._run())
|
||||
assert "old.xml -> new.xml" in message
|
||||
assert " R ; old.xml -> new.xml" in message
|
||||
|
||||
def test_custom_separator(self) -> None:
|
||||
change_set = ChangeSet(
|
||||
added=[ChangeRecord(path="pump.xml", change_type=ChangeType.ADDED)],
|
||||
deleted=[ChangeRecord(path="gone.xml", change_type=ChangeType.DELETED)],
|
||||
)
|
||||
run = self._run()
|
||||
message = build_commit_message(change_set, run, file_separator="|")
|
||||
assert " A | pump.xml ; UNKNOWN ; " not in message
|
||||
assert " A | pump.xml | UNKNOWN | " in message
|
||||
assert " D | gone.xml" in message
|
||||
|
||||
Reference in New Issue
Block a user