Compare commits
2
Commits
75e8e91f9a
...
4634048eab
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
4634048eab | ||
|
|
ceaa7d18d8 |
@@ -115,6 +115,9 @@ A detail that is set is used and not asked; the summary marks it with
|
||||
default to no: create now, reusing an existing repository, an existing
|
||||
directory, `core.hooksPath` and replacing a template file.
|
||||
- These keys are accepted in `config.env` only, never in `.env`.
|
||||
- A value is read as plain text: an unquoted ` #` starts a comment and cuts the
|
||||
value there. Put a description that contains ` #` in double quotes, for
|
||||
example `PROJECT_DESCRIPTION="Tool for #mirrors"`.
|
||||
|
||||
With all eight set, a run asks only the confirmations:
|
||||
|
||||
@@ -144,7 +147,8 @@ src/create-project.sh --config /path/to/config.env --env /path/to/.env
|
||||
|
||||
The script asks for, in this order: repository name, description, visibility,
|
||||
Gitea owner, whether to also create a GitHub repository (and its owner), the
|
||||
local directory and whether to enable the plan gate. It then checks both hosts
|
||||
local directory and whether to enable the plan gate (a detail set in
|
||||
[`config.env`](#configenv-project-details-optional) is not asked). It then checks both hosts
|
||||
with read-only requests and prints a plan:
|
||||
|
||||
```text
|
||||
|
||||
+2
-1
@@ -35,7 +35,8 @@ GITEA_API_URL=https://<your gitea instance>/api/v1
|
||||
# empty. An invalid value stops the run and names the key. Remove or comment
|
||||
# out a line to be asked for it. The confirmations ("Create these now" and
|
||||
# the questions about existing repositories, directories and files) are
|
||||
# always asked.
|
||||
# always asked. Put a value that contains " #" in double quotes: an unquoted
|
||||
# " #" starts a comment.
|
||||
#PROJECT_NAME=my-project
|
||||
#PROJECT_DESCRIPTION=What the project is for
|
||||
#PROJECT_VISIBILITY=private # private or public
|
||||
|
||||
@@ -23,7 +23,7 @@ document of a type. `Primary File` may contain a glob (e.g.
|
||||
| DM | Domain Model | docs/domain-model.md | 003 |
|
||||
| DICT | Domain Dictionary (PO and IT terms) | docs/dictionary.md | 002 |
|
||||
| UCD | Use Case Diagram | docs/use-case-diagram.md | 002 |
|
||||
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 019 |
|
||||
| RC | SQA Review Record | docs/sqa/reviews/rc-*.md | 020 |
|
||||
| TM | Traceability Matrix | docs/sqa/traceability-matrix.md | 002 |
|
||||
|
||||
## Languages
|
||||
|
||||
@@ -0,0 +1,70 @@
|
||||
# SQA Review Record: Shell code review of the MIL-004 change
|
||||
|
||||
## Metadata
|
||||
| Key | Value |
|
||||
| --- | --- |
|
||||
| ID | RC-019 |
|
||||
| CrossReference | [MIL-004], [QC-SH-001], [RC-016] |
|
||||
|
||||
## Version History
|
||||
| Date | Status | Author | Reviewer | Change | Commit |
|
||||
| --- | --- | --- | --- | --- | --- |
|
||||
| 2026-10-05 | Proposed | Jens Tirsvad Nielsen | S02 | Initial version | [ceaa7d1] |
|
||||
|
||||
---
|
||||
|
||||
## Artifact Under Review
|
||||
|
||||
- Instance reviewed: the MIL-004 change to `src/create-project.sh` and `src/lib/` (`config.sh`, `constants.sh`, `project.sh`), `tests/test-presets.sh`, the README and `config.env.example`, on branch `mil-004-configurable-details` (commit `75e8e91` plus the fixes listed below), tasks 1 to 5 (issues #27 to #31) of [MIL-004].
|
||||
- Checklist used: [QC-SH-001]. The rest of the code was reviewed in [RC-016].
|
||||
- Review date: 2026-10-05
|
||||
- Tool versions: bash 5.2.37, shellcheck 0.11.0, shfmt 3.14.1 (Windows, Git Bash)
|
||||
|
||||
## Checklist Results
|
||||
|
||||
| # | Criterion | Status | Evidence/Notes |
|
||||
| --- | --- | --- | --- |
|
||||
| 1 | Starts with `#!/usr/bin/env bash` and `set -euo pipefail` | Pass | Unchanged. |
|
||||
| 2 | Every expansion is quoted; lists are arrays; tests use `[[ ]]` and `$(...)` | Pass | `shellcheck` is clean. Values from `config.env` are only ever compared, matched against validators or printed; none reaches `eval`, a command line or a file name before it passed a validator. |
|
||||
| 3 | Names follow the conventions | Pass | `check_preset`, `preset_detail`, `source_note` and the `HINT_*` constants follow the rules. See finding F4 on the name `PROJECT_NAME`. |
|
||||
| 4 | Passes `shellcheck` and `bash -n` with no unexplained `disable` comments | Pass | No new `disable`. |
|
||||
| 5 | Errors go to standard error with an `error:` message and a non-zero exit code | Pass | Every refusal goes through `die`, names the key and the file, never the value. |
|
||||
| 6 | Temporary files use `mktemp` with a `trap` cleanup | Pass | Not touched. |
|
||||
| 7 | No secret is written in the script, echoed, or put on a command line | Pass | The new keys carry no credential; they are rejected in `.env`, and credential keys stay rejected in `config.env` (tests). The description is printed in the summary, as it was when asked. |
|
||||
| 8 | A script that changes state defaults to a dry run | Pass | Unchanged: a preset never skips the dry run or "Create these now". Tests prove that with all eight keys set, an empty or missing answer creates nothing. |
|
||||
| 9 | A header comment states purpose, usage, options, environment variables and exit codes | Pass | Fixed during this review: the header said only "asks for the project details"; it now says that details set in `config.env` are not asked. |
|
||||
| 10 | The script implements a task or design it cites; deviations are recorded | Pass | Tasks 1 to 5 of [MIL-004] and extensions 3a and 3b of [UC-001]. Deviations: values are accepted in any case, and `USE_GITHUB`/`ENABLE_PLAN_GATE` take `yes` or `no` only; both are in the README. |
|
||||
| 11 | Behaviour is tested for success, failure and any disabled or bypass path | Pass | 919 checks in the full suite before the review, 0 failed. New: each key set, absent, empty and invalid; mixed asked and preset; `USE_GITHUB` interplay; the summary marker; no prompt text for a preset; the confirmations. Mutation check: making the preset lookup always fail made the tests fail. |
|
||||
| 12 | Formatted with `shfmt` | Pass | No difference. |
|
||||
| 13 | Safe to re-run | Pass | No state is kept. |
|
||||
| 14 | Bash version and external tools stated | Pass | Unchanged. |
|
||||
|
||||
## Findings
|
||||
|
||||
| # | Finding | Severity | Status |
|
||||
| --- | --- | --- | --- |
|
||||
| F1 | An unquoted `PROJECT_DESCRIPTION` containing ` #` is silently cut at the comment mark (`Tool # for mirrors` becomes `Tool`), with no message. This is how the parser reads every value, but a description is the one free-text key, so it is where it bites. A value with both kinds of quote cannot be set at all (it can still be typed at the prompt). | Low (surprise, no data loss or security effect) | Fixed: the README and `config.env.example` say to put such a value in double quotes; a test pins both behaviours. The both-quotes case is documented as a limit of the parser. |
|
||||
| F2 | The header of `create-project.sh` and the README sentence "The script asks for, in this order" did not mention that preset details are not asked. | Low (documentation) | Fixed. |
|
||||
| F3 | The summary marks the source after the value, so a fully preset run reads `my-app (from config.env) (private (from config.env))`. Correct but noisy, and `USE_GITHUB=yes` is not marked on the GitHub line (only the owner is). | Low (readability) | Accepted: the marker is asserted by the tests and the wording is not a requirement; revisit if the Maintainer wants a table layout. |
|
||||
| F4 | The constant `PROJECT_NAME` (the name of this tool, `RepoFoundry`) and the config key `PROJECT_NAME` (the name of the new project) share a spelling. They never meet in code (the key lives in `CONFIG`), but a reader can confuse them. | Low (maintainability) | Open: renaming the constant is out of scope for this change; a candidate for a later clean-up. |
|
||||
| F5 | A `config.env` that sets `PROJECT_NAME` and `GITEA_OWNER` makes every run use them. Existing repositories are still detected and need the reuse confirmation, so nothing is overwritten. | Info | Documented in the README (per-project configuration). |
|
||||
|
||||
No finding affects credentials, ownership, the mirror direction or the confirmations.
|
||||
|
||||
## Overall Verdict
|
||||
|
||||
Go — all mandatory criteria pass after the fixes for F1 and F2 (found and fixed during this review; the test for F1 was added; the full suite was rerun afterwards: 923 checks, 0 failed). F3 and F4 are recorded and not blocking. Author and reviewer are the same person for now (S01 and S02 are both held by the Maintainer), so the framework independence rule is not met; re-review when a second person takes S02.
|
||||
|
||||
## Action Items
|
||||
|
||||
| Action | Owner | Due |
|
||||
| --- | --- | --- |
|
||||
| Decide whether to rename the constant `PROJECT_NAME` (F4) and whether to restyle the summary markers (F3) | S02 | 2026-10-30 |
|
||||
|
||||
---
|
||||
|
||||
[MIL-004]: ../../milestones/mil-004-configurable-details.md
|
||||
[UC-001]: ../../uc-001/uc.md
|
||||
[RC-016]: ./rc-016-create-project-sh.md
|
||||
[QC-SH-001]: ../../../framework/qc/qc-programming-shell.md
|
||||
[ceaa7d1]: https://git.tirsystem.com/TirSystem-BashScript/repo_foundry/commit/ceaa7d18d8908b4d1e3fb089f238c99b883d71e0
|
||||
@@ -30,7 +30,7 @@ updated whenever an artifact instance is created or reviewed.
|
||||
| [MIL-001] | MIL | [BC-001], [PP-001] | [US-001] | [RC-011], [RC-016] |
|
||||
| [MIL-002] | MIL | [BC-001], [PP-001] | [US-001] | [RC-014], [RC-017] |
|
||||
| [MIL-003] | MIL | [BC-001], [PP-001] | [US-001] | [RC-015], [RC-017] |
|
||||
| [MIL-004] | MIL | [BC-001], [PP-001] | [US-001] | [RC-018] |
|
||||
| [MIL-004] | MIL | [BC-001], [PP-001] | [US-001] | [RC-018], [RC-019] |
|
||||
| [UCD-001] | UCD | [BC-001], [SA-001] | [US-001], [UC-001] | [RC-009] |
|
||||
| [US-001] | US | [BC-001], [UCD-001], [MIL-001], [MIL-002], [MIL-003], [MIL-004] | [UC-001] | [RC-001] |
|
||||
| [UC-001] | UC | [UCD-001], [US-001], [SA-001] | [SSD-001], [DM-001] | [RC-002] |
|
||||
@@ -56,6 +56,7 @@ updated whenever an artifact instance is created or reviewed.
|
||||
[MIL-003]: ../milestones/mil-003-scaffold-and-release.md
|
||||
[MIL-004]: ../milestones/mil-004-configurable-details.md
|
||||
[RC-018]: ./reviews/rc-018-mil-004.md
|
||||
[RC-019]: ./reviews/rc-019-mil-004-code.md
|
||||
[UCD-001]: ../use-case-diagram.md
|
||||
[US-001]: ../user-stories.md
|
||||
[UC-001]: ../uc-001/uc.md
|
||||
|
||||
@@ -5,7 +5,7 @@
|
||||
# RepoFoundry creates a Gitea repository, optionally an empty GitHub
|
||||
# repository with a Gitea -> GitHub push mirror, and a local project with
|
||||
# the SQA-QC-Framework. It validates the configuration and credentials,
|
||||
# asks for the project details, checks both hosts with read-only requests
|
||||
# asks for the project details (those set in config.env are not asked), checks both hosts with read-only requests
|
||||
# (tokens, owners, names, license, SSH) and, with --apply, creates the
|
||||
# repositories and the mirror, then the local project: its directory, git
|
||||
# repository, remotes (no credential in any address), the framework as a
|
||||
|
||||
@@ -262,3 +262,14 @@ test_with_every_detail_set_an_existing_directory_is_not_replaced() {
|
||||
assert_file_exists "existing file kept" "$WORK/my-app/mine.txt"
|
||||
assert_eq "content kept" "keep" "$(cat "$WORK/my-app/mine.txt")"
|
||||
}
|
||||
|
||||
test_a_quoted_description_may_contain_a_hash() {
|
||||
local answers=$'my-app\npublic\nTirSystem\nn\n\nn\n'
|
||||
collect_with 'PROJECT_DESCRIPTION="Tool for #mirrors"' "$answers"
|
||||
assert_status "quoted" 0 "$STATUS"
|
||||
assert_contains "whole value kept" "$OUT" "description=Tool for #mirrors"
|
||||
# Unquoted, the same text is cut at the comment mark, as documented.
|
||||
collect_with 'PROJECT_DESCRIPTION=Tool for #mirrors' "$answers"
|
||||
assert_contains "cut at the comment" "$OUT" "description=Tool for"
|
||||
assert_not_contains "comment dropped" "$OUT" "mirrors"
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user