feat: add idea validation script and GitHub Actions workflow#33
Conversation
- validate-idea.sh: checks template structure, required sections, and unfilled placeholders - validate-idea.yml: triggers on PRs adding ideas/*.md, validates PR title and file format Signed-off-by: Sinduri Guntupalli <sinduri.guntupalli@dynatrace.com>
Signed-off-by: Sinduri Guntupalli <sinduri.guntupalli@dynatrace.com>
KatharinaSick
left a comment
KatharinaSick
left a comment
There was a problem hiding this comment.
A few minor comments. Thank you 😊
| steps: | ||
|
|
||
| # Check PR title before checkout — fast fail with no runner cost. | ||
| - name: Check PR title format |
There was a problem hiding this comment.
I think we don't need to be that strict on the PR title to not annoy people :)
There was a problem hiding this comment.
Yes, I agree. Removed the check.
|
|
||
| # fetch-depth: 0 is required so git diff can compare against the base branch. | ||
| - name: Checkout repository | ||
| uses: actions/checkout@v4 |
There was a problem hiding this comment.
v6 is already available: https://github.com/actions/checkout
I also ran into that trap while vibe coding - it's not always using the latest versions 😅
There was a problem hiding this comment.
Hehehe, fixed it now.
| - name: Make validation script executable | ||
| run: chmod +x scripts/validate-idea.sh | ||
|
|
||
| # Only validate files *added* by this PR (not modified/deleted). |
There was a problem hiding this comment.
Why only added? I think if an idea is modified, it would be great to check it as well?
(could happen if the idea author has an idea for an additional level e.g.)
There was a problem hiding this comment.
Good Point. Updated to both Added and Modified.
| # Strict mode | ||
| # --------------------------------------------------------------------------- | ||
| # -e is intentionally NOT set — we want all checks to run and collect every | ||
| # failure before exiting, so the contributor sees everything at once. |
| # Check 1 — File guard | ||
| # --------------------------------------------------------------------------- | ||
| # The workflow path filter handles this in CI, but local runs have no filter. | ||
| [[ "$FILE" == "ideas/adventure-idea-template.md" ]] \ |
There was a problem hiding this comment.
why is it actually not ok to dot hat? It would be nice after adjusting the template (e.g. changing wording)
There was a problem hiding this comment.
The template file had placeholders and it wouldn't have passed the check. But I agree with your point, so adjusted that the template file is checked but the placeholders check will be skipped.
| # --------------------------------------------------------------------------- | ||
| ERRORS_BEFORE=${#ERRORS[@]} | ||
|
|
||
| grep -qE "^### (🟢|🟡|🔴)" "$FILE" \ |
There was a problem hiding this comment.
I would not check for specific emojis - if it's a single level adventure or if there are more than 3 levels the emojis might differ
There was a problem hiding this comment.
True. Removed emoji specific check.
KatharinaSick
left a comment
KatharinaSick
left a comment
There was a problem hiding this comment.
I really like how you did this checker but when revisiting the new-adventure script, I feel like there are a couple of checks missing - e.g. level metadata: https://github.com/dynatrace-oss/open-ecosystem-challenges/blob/main/scripts/new-adventure.sh#L23-L26
The more I think about it, the more I think if a dry run version of the new-adventure script wouldn't be easier than a separate script. But I also don't want to make you redo all the work -> your choice if we stay with 2 scripts or combine them into one with a dry-run option to test parsing. I think the level. metadata check is the most important thing we're missing
Signed-off-by: Sinduri Guntupalli <sinduri.guntupalli@dynatrace.com>
- Add idea-parser.sh with parse_adventure_header(), parse_level_heading(), extract_overview_field(), extract_level_section(), extract_level_description() - Add ADVENTURE_HEADER_PATTERN and LEVEL_HEADING_PATTERN constants - Update validate-idea.sh: use shared lib for all extraction and validation, add adventure header format check, add per-level subsection content checks, validate Theme/Technologies have content (not just presence) - Update new-adventure.sh: source shared lib, remove all inline parsing Signed-off-by: Sinduri Guntupalli <sinduri.guntupalli@dynatrace.com>
Rethought this added a idea-parser file that is a shared lib for all extraction and validation to be used in both new-adventure and validate-idea files |
KatharinaSick
left a comment
KatharinaSick
left a comment
There was a problem hiding this comment.
Looks good - thank you! :)
2 participants
What does this PR do?
Type of PR
📝 Other