refactor: add skills/ build system with CI freshness check#4
Conversation
Migrate from manually editing skillshare-hub.json to a structured build system: - Add skills/ directory as source of truth (skills/core.json) - Add scripts/build.sh to generate skillshare-hub.json from skills/*.json - Update Makefile with build target (make build/validate/audit/ci) - Update CI to build from skills/ and verify hub.json is in sync - Refactor scripts/audit.sh: group by repo to deduplicate clones, Bash 3.x compat (temp dir instead of associative arrays), clean up leftover temp dirs before/after clone - Update CONTRIBUTING.md with new workflow and categories - Update PR template checklist
|
No new or changed skill sources to audit. |
Summary of ChangesHello @runkids, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request introduces a significant refactoring of the skill management system, transitioning from a single, manually edited JSON file to a modular, build-driven approach. This change aims to enhance the maintainability and scalability of skill definitions, streamline the contribution process for new skills, and ensure data consistency through automated CI checks. The update also includes optimizations to the skill auditing script, making the overall system more robust and efficient. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Ignored Files
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This is a great refactoring that significantly improves the contribution workflow and maintainability of the skill hub. The move to a structured skills/*.json build system is a solid architectural change. The updated audit.sh script is much more efficient by avoiding duplicate clones, and the attention to Bash 3.x compatibility is a nice touch. The documentation updates are clear and will be very helpful for contributors.
I've left a couple of suggestions on the shell scripts to improve their robustness, one regarding a bug in file checking and another on temporary file cleanup. Overall, this is an excellent pull request.
| if [ ${#files[@]} -eq 0 ]; then | ||
| echo "ERROR: No .json files found in $SKILLS_DIR" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
This check for missing .json files is not quite correct for standard bash behavior. Without shopt -s nullglob, if no files match a glob pattern, the pattern itself is returned as a single-element array. This means ${#files[@]} will be 1, not 0, and this check will be bypassed, leading to a less clear error from jq later.
A more reliable way to check is to see if the first element of the array actually exists as a file.
| if [ ${#files[@]} -eq 0 ]; then | |
| echo "ERROR: No .json files found in $SKILLS_DIR" | |
| exit 1 | |
| fi | |
| if [ ! -e "${files[0]}" ]; then | |
| echo "ERROR: No .json files found in $SKILLS_DIR" | |
| exit 1 | |
| fi |
| group_dir=$(mktemp -d) | ||
| trap 'rm -rf "$group_dir"' EXIT |
There was a problem hiding this comment.
This script creates temporary directories in multiple places (group_dir and clone_dir). While group_dir is cleaned up by a trap, the clone_dirs created in /tmp are removed manually. If the script is interrupted (e.g., with Ctrl-C), these clone directories might be left behind.
For more robust cleanup, consider creating a single parent temporary directory for all transient data. A single trap on this parent directory would ensure all temporary files and directories are cleaned up, regardless of how the script terminates.
Example structure:
# Create a single root temp dir at the start
audit_tmp_dir=$(mktemp -d)
trap 'rm -rf "$audit_tmp_dir"' EXIT
# Use subdirectories within it
group_dir="${audit_tmp_dir}/groups"
mkdir "$group_dir"
# ...
clone_dir="${audit_tmp_dir}/clones/${safe_name}"
# ...This would make the script's cleanup mechanism more resilient.
|
No new or changed skill sources to audit. |
1 participant
Summary
skillshare-hub.jsonto a structuredskills/*.jsonbuild systemscripts/build.shto generateskillshare-hub.jsonfromskills/*.json(sorted, formatted)skillshare-hub.jsonis in sync (git diff --quiet)scripts/audit.sh: group sources by repo to deduplicate clones, Bash 3.x compat (temp dir instead of associative arrays), clean up leftover temp dirsCONTRIBUTING.mdwith new workflow, categories table, and local dev commandsChanges
skills/skill.jsonscripts/build.shskills/*.json→skillshare-hub.jsonscripts/audit.shMakefilebuildtarget,cinow runsbuild → validate → audit.github/workflows/validate-pr.ymlskills/**trigger, build step, freshness checkCONTRIBUTING.md.github/PULL_REQUEST_TEMPLATE.mdTest plan
make buildproduces identicalskillshare-hub.jsonto mainmake validatepasses