Skip to content

Conversation

@Pertempto
Copy link
Contributor

@Pertempto Pertempto commented Nov 18, 2025

Changes

  • Added specs/ directory with my current todo list for the project. These will definitely be revised with time as I figure out what I'm doing.
  • Added super simple CONTRIBUTING.md and AGENTS.md.
  • Added specs/README.md explaining Specture

@Pertempto Pertempto requested a review from bambam955 November 18, 2025 17:27
@Pertempto
Copy link
Contributor Author

@bambam955 no rush on this one. I just wanted to write it all down before I take a break from the project for a bit.

@github-actions

This comment was marked as outdated.

@github-actions
Copy link

github-actions bot commented Nov 18, 2025

Changes Requested

Please address the following before merging:

  • Normalize frontmatter metadata: pick a single date field and apply it everywhere. Recommendation: use created: YYYY-MM-DD. Update the specs/README.md example (replace creation_date) and add created: to specs/000-mvp.md, specs/001-consistent-demo-app-name.md, and specs/002-update-demo-app-dependencies.md.
  • Clarify intent/scope for specs/000-mvp.md: add a frontmatter field (e.g., scope: high-level or scope: actionable) so humans and automation know how to treat it.
  • Add minimal, testable acceptance criteria for each top-level Task List item in specs/000-mvp.md (measurable checks like example CAN frame IDs, update rates, tolerances, or simple verification steps).
  • Expand and correct terminology: in specs/000-mvp.md expand VHAL on first use to Vehicle Hardware Abstraction Layer (VHAL) and fix any Vehical -> Vehicle typos.
  • Update CONTRIBUTING.md to require Conventional Commit-style PR titles, include a one-line example, and link to https://www.conventionalcommits.org/.

Summary of Changes

  • Added/updated docs/specs and contrib files:
    • specs/000-mvp.md (new)
    • specs/001-consistent-demo-app-name.md (new)
    • specs/002-update-demo-app-dependencies.md (new)
    • specs/README.md (new)
    • CONTRIBUTING.md (new)
    • AGENTS.md (new)
    • README.md (updated)

Overall Feedback

  • Negative: The PR introduces useful spec scaffolding but has metadata inconsistencies (frontmatter date fields), missing acceptance criteria in the MVP spec, and a few terminology/typo issues. These gaps will make it harder for automation and contributors (human or agent) to verify work unambiguously.

  • Positive: This is a thoughtful, well-structured start to a spec system. The Specture-based README is clear and the numeric precedence idea is neat. The task-list style and examples will make future collaboration and tooling much easier. Nice work — this will scale well once the frontmatter and acceptance-criteria items are fixed. 🚀📚

@Pertempto — ping me after you push the frontmatter/date and CONTRIBUTING updates and I will re-check quickly.

@Pertempto
Copy link
Contributor Author

Add minimal, testable acceptance criteria / verification steps for each top-level Task List item in specs/ (examples for CAN frames, expected VHAL attributes, timing/accuracy tolerances).

The requirements I was given weren't this specific, so I'm not going to be that specific in the spec.

@Pertempto Pertempto marked this pull request as draft December 16, 2025 21:21
@Pertempto
Copy link
Contributor Author

Pertempto commented Dec 16, 2025

Marking as draft until I can document the spec system and link it in the README. Also I plan to remove the opencode commands. I've found it more useful to put information in AGENTS.md so that it can be used across agents. I don't like the idea of getting locked in to one specific agent system.

@bambam955
Copy link
Contributor

I would agree with removing the custom opencode commands. I personally don't use opencode much anymore so if I were to help develop this project I'd have to either change my development workflow or add copies of those commands to the list for whatever agent I am using.

@Pertempto
Copy link
Contributor Author

I need to add a link to this repo: https://github.com/specture-system/specture

Right now it just contains a basic README, in the future I plan to include more docs and tooling.

@github-actions

This comment was marked as resolved.

@github-actions

This comment was marked as resolved.

@Pertempto
Copy link
Contributor Author

Request: please confirm whether 000-mvp is intended as an epic that will span multiple PRs.

No epics in the Specture system. I have clarified this.

@mrs-electronics-inc mrs-electronics-inc deleted a comment from github-actions bot Dec 18, 2025
@Pertempto Pertempto requested a review from bambam955 December 18, 2025 18:45
@Pertempto Pertempto marked this pull request as ready for review December 18, 2025 18:45
Co-authored-by: Addison Emig <addison.emig@mrs-electronics.com>
Signed-off-by: Bennett Moore <120052232+bambam955@users.noreply.github.com>
Copy link
Contributor

@bambam955 bambam955 left a comment

Choose a reason for hiding this comment

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

Overall I really like this approach to feature tracking. It's definitely taking some mental rewiring to get used to but I really appreciate that it focuses more on the "why" for a feature and avoids tons of implementation details.

I left some comments: mostly grammatical/wording, plus some notes about legit system revisions.

@Pertempto
Copy link
Contributor Author

Pertempto commented Dec 18, 2025

One major flaw with the current design of Specture - there is no way to track who is actually implementing the change. There is an author field, but maybe there should also be an optional implementer or assignee field? Really it should probably support a list of people. The author field should as well. A large epic-sized spec could very well be drafted up by a collaboration of multiple people.

Long term, there could be CI/CD tooling that blocks merge until a spec has the appropriate fields and structure 👀

Like no merging a spec with status: approved until a maintainer approved the PR.

@bambam955
Copy link
Contributor

One major flaw with the current design of Specture - there is no way to track who is actually implementing the change. There is an author field, but maybe there should also be an optional implementer or assignee field?

I think there should definitely be an approved field. But not an assignee field. In my opinion most specs, particularly more high-level ones, will be implemented by many different people. Placing the restriction that one person is assigned to one spec seems to be way too analogous to implementation-oriented systems, which this is not.

@bambam955
Copy link
Contributor

bambam955 commented Dec 18, 2025

Long term, there could be CI/CD tooling that blocks merge until a spec has the appropriate fields and structure 👀

Like no merging a spec with status: approved until a maintainer approved the PR.

This is a GREAT idea. I hope you are documenting all of these in the Specture repo.

It would also be cool to have tooling that lists all of the contributors to a spec. Git should make this easy under the hood, something like git log --pretty="%an" -- <file_path> | sort | uniq

@Pertempto
Copy link
Contributor Author

Pertempto commented Dec 18, 2025

I think there should definitely be an approved field. But not an assignee field.

approved field? That is confusing when there is an approved status. Maybe approved_by?

In my opinion most specs, particularly more high-level ones, will be implemented by many different people. Placing the restriction that one person is assigned to one spec seems to be way too analogous to implementation-oriented systems, which this is not.

Fair. My one fear was two people working on implementing the same spec in parallel. But really that shouldn't be a big deal. Good team communication will avoid it and even if you do end up with two implementations that's not really a bad thing. You can combine the best of both and end up better for it.

@bambam955
Copy link
Contributor

approved field? That is confusing when there is an approved status. Maybe approved_by?

Yes that is what I meant, sorry. And maybe also created should be created_at.

@Pertempto Pertempto requested a review from bambam955 December 18, 2025 20:21
@Pertempto
Copy link
Contributor Author

Oops push initially failed. Everything should be up now.

@github-actions
Copy link

specs/README.md: Frontmatter field names are inconsistent with the spec files. You currently use creation_date in the README example but the spec files don't include any date field.

  • Action: pick a single canonical frontmatter field name (created or created_at or creation_date) and update the README example to match.
  • Action: add the chosen field to all spec files' frontmatter (specs/000-mvp.md, specs/001-consistent-demo-app-name.md, specs/002-update-demo-app-dependencies.md) so metadata is consistent and machine-readable.

Suggestion: use created: YYYY-MM-DD (short and common) and document it in the README under Frontmatter.

@Pertempto Pertempto requested a review from bambam955 December 18, 2025 20:53
@Pertempto Pertempto merged commit fea1725 into main Dec 18, 2025
1 check passed
@Pertempto Pertempto deleted the ae-add-basic-specs branch December 18, 2025 20:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants