Update the recipe files.#132
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the build system by removing Docker-related configurations, restructuring LaTeX source files into a dedicated directory, and simplifying the bootstrap.sh script. Feedback highlights critical issues in the new bom target regarding path resolution and virtual environment activation. Furthermore, the removal of OS detection in bootstrap.sh breaks required OpenBSD support. Other recommendations include cleaning up .PHONY targets in the Makefile, removing an unused variable, and simplifying the autotools execution sequence.
| . $(PY39)/bin/activate && \ | ||
| pip install -r .admin/src/requirements.txt && \ | ||
| python .admin/src/generate_bom.py -d ../recipes -o ../BOM.md |
There was a problem hiding this comment.
The bom target contains several issues that will cause it to fail:
- Incorrect Activation: It attempts to source
$(PY39)/bin/activate. However,PY39is defined inconfigure.acas the Python interpreter name (e.g.,python3.9), not the virtual environment directory. It should use$(VENV)(which is.venv). - Path Resolution: The paths
.admin/src/requirements.txtand.admin/src/generate_bom.pyare incorrect. Since this Makefile is already located within the.admin/directory, these paths will resolve to.admin/.admin/.... They should be relative to the Makefile's location. - Best Practice: It is recommended to use the venv-specific binaries (e.g.,
$(VENV)/bin/python) to ensure the correct environment is used, consistent with thepythontarget implementation.
. $(VENV)/bin/activate && \
$(VENV)/bin/python -m pip install -r src/requirements.txt && \
$(VENV)/bin/python src/generate_bom.py -d ../recipes -o ../BOM.md
| check_docker | ||
| detect_os | ||
| #check_installed doxygen | ||
| required_files |
There was a problem hiding this comment.
The removal of the detect_os call (and its definition) breaks OpenBSD support. The MY_OS variable will now remain at its default value of "unknown", which causes the script to skip the necessary environment variable overrides (e.g., AUTOCONF_VERSION=2.71) required for OpenBSD in functions like run_aclocal. Given that the repository style guide (GEMINI.md, line 33) explicitly mentions OpenBSD support, this detection logic should be preserved.
| @@ -5,6 +5,11 @@ PYTHON = $(VENV)/bin/python3 | |||
|
|
|||
| .PHONY: clean docker python | |||
| echo "Running libtoolize..." | ||
| libtoolize --force --copy --automake || exit 1 | ||
| } | ||
| DEB_PKG=(fig2dev gnuplot) |
| run_aclocal | ||
| run_autoconf | ||
| run_automake |
There was a problem hiding this comment.
The explicit calls to run_aclocal, run_autoconf, and run_automake are redundant. The run_autoconf function invokes autoreconf -i, which is designed to automatically run all necessary autotools (aclocal, autoconf, autoheader, automake, etc.) in the correct sequence. You can simplify the main function by relying on autoreconf.
| run_aclocal | |
| run_autoconf | |
| run_automake | |
| run_autoconf |
|
/gemini review |
|
/gemini summary |
There was a problem hiding this comment.
Code Review
This pull request refactors the project's build and maintenance infrastructure, transitioning from a Docker-centric setup to a local Autotools and Python-based workflow. Significant changes include the removal of Docker-related files, the simplification of the bootstrap.sh script by removing OS-specific installation logic, and the addition of a Python script that uses the Gemini API to automatically populate recipe metadata. Furthermore, the Bill of Materials generation was updated, and recipe files were standardized with YAML frontmatter. Review feedback highlights several critical issues: the Makefile.am violates the repository style guide by removing the PY39 variable and hardcoding the virtual environment path to /tmp, which poses multi-user risks. Additionally, the new AI script uses an invalid model identifier and contains a prompt that could lead to JSON parsing failures. There are also inconsistencies in the Makefile regarding virtual environment execution and unused variables in the bootstrap script.
| bom: | ||
| . $(VENV)/bin/activate && \ | ||
| $(PYTHON) -m pip install -r src/requirements.txt && \ | ||
| $(PYTHON) src/generate_bom.py -d ../recipes -o ../BOM.md |
There was a problem hiding this comment.
The generate_bom.py script has been updated and no longer accepts a -d argument; it now uses -b (or --base-dir) which defaults to ... Furthermore, using $(PYTHON) (the system Python) inside the activation block is incorrect as it bypasses the virtual environment's interpreter and its installed dependencies. You should use the Python executable from the virtual environment.
$(VENV)/bin/python src/generate_bom.py -b .. -o ../BOM.md
| try: | ||
| # New SDK syntax for content generation | ||
| response = client.models.generate_content( | ||
| model='gemini-2.5-flash', |
| ACLOCAL_AMFLAGS = -I config/m4 -I aclocal | ||
| VENV = .venv | ||
| PYTHON = $(VENV)/bin/python3 | ||
| VENV = /tmp/venv |
There was a problem hiding this comment.
Hardcoding the virtual environment to /tmp/venv can lead to permission issues or collisions in multi-user environments. It is generally better to keep the virtual environment local to the project directory (e.g., .venv). Additionally, the removal of the PY39 variable violates the repository style guide.
References
- Always use the $(PY39) variable defined in the Makefile for virtual environments. (link)
| echo "Running libtoolize..." | ||
| libtoolize --force --copy --automake || exit 1 | ||
| } | ||
| DEB_PKG=(fig2dev gnuplot) |
| {{ | ||
| "title": "Recipe Name", | ||
| "author": "devsecfranklin", | ||
| "prep_time": 15, // integer in minutes (guess if not explicit) | ||
| "cook_time": 30, // integer in minutes (guess if not explicit) | ||
| "yield": "4 servings", | ||
| "difficulty": "medium", // low, medium, or high | ||
| "tags": ["tag1", "tag2"], // e.g., breakfast, dessert, vegan, high-availability | ||
| "hardware": ["skillet", "mixing bowl"] | ||
| }} |
There was a problem hiding this comment.
The prompt includes // comments within the JSON structure. Since standard JSON does not support comments, json.loads() will fail if the model includes them in the response. It is recommended to remove these comments from the prompt example.
| {{ | |
| "title": "Recipe Name", | |
| "author": "devsecfranklin", | |
| "prep_time": 15, // integer in minutes (guess if not explicit) | |
| "cook_time": 30, // integer in minutes (guess if not explicit) | |
| "yield": "4 servings", | |
| "difficulty": "medium", // low, medium, or high | |
| "tags": ["tag1", "tag2"], // e.g., breakfast, dessert, vegan, high-availability | |
| "hardware": ["skillet", "mixing bowl"] | |
| }} | |
| {{ | |
| "title": "Recipe Name", | |
| "author": "devsecfranklin", | |
| "prep_time": 15, | |
| "cook_time": 30, | |
| "yield": "4 servings", | |
| "difficulty": "medium", | |
| "tags": ["tag1", "tag2"], | |
| "hardware": ["skillet", "mixing bowl"] | |
| }} |
Summary of ChangesThis pull request standardizes the recipe repository by implementing a strict YAML frontmatter schema across all markdown files. It includes new tooling to automate the population of this metadata using AI, alongside significant cleanup of the build system and directory structure to improve maintainability and support automated documentation generation. Highlights
Ignored Files
Activity
|
YAML Frontmatter Schema Definition
Defining a strict YAML schema ensures predictable parsing for automated tooling, such as Bill of Materials (BOM) generation and document rendering. The frontmatter must be placed at the very beginning of the
.mdfile, enclosed by---delimiters.The ingredients and step-by-step instructions should remain in the standard markdown body to leverage native rendering, while the YAML block handles all structured metadata.
Schema Structure and Data Types
title: (String) The exact name of the recipe.author: (String) The handle of the contributor (e.g., devsecfranklin).date: (Date: YYYY-MM-DD) The original submission or last modification date.prep_time: (Integer) Preparation time required, defined in minutes.cook_time: (Integer) Active or passive cooking time, defined in minutes.yield: (String) Total servings or output volume.difficulty: (String) Categorical variable (e.g., low, medium, high).tags: (List of Strings) Classifications for indexing and searching.hardware: (List of Strings) Required tools or kitchen equipment to execute the recipe.Implementation Template
Create a skeleton file at
.admin/templates/recipe_skel.mdcontaining the following template:To insert this schema into an existing recipe file, open the target file using
viand execute the following command in normal mode to read the template into the top of the current buffer:This structure strictly organizes the metadata and ensures that the downstream Python parsers can extract the necessary fields for BOM creation without encountering markdown cross-reference markers.