Conversation
- Created unit-testing.md to provide guidelines on when and how to write unit tests, including examples and best practices. - Added vitest-setup.md for one-time Vitest configuration instructions, including installation, configuration, and usage. - Updated .gitignore to exclude unnecessary files and directories. - Introduced AGENTS.md to outline the skills available for development tasks and the process for using them. - Added CLAUDE.md to provide an overview of the project, key technologies, setup commands, and development workflow. - Created SETUP-AEM-EDS.md to explain the setup of AEM EDS projects with multiple authoring options. - Enhanced README.md to reflect the new project structure, authoring environments, and development workflow.
Summary of ChangesHello @tusharmane-tm, 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 represents a significant architectural and operational shift, integrating a robust set of AI agent skills to streamline development and content migration processes for AEM Edge Delivery Services. It formalizes workflows for building and reviewing code, managing content, and importing pages, all while introducing new UI components tailored for event-based websites. The changes aim to enhance efficiency, maintainability, and author experience across the project. 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
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 pull request introduces a new "skills" framework for an AI agent, including documentation, discovery scripts, and various helper scripts for interacting with AEM documentation and block repositories. It also adds several new blocks for a conference website solution and updates the project's README and other documentation to reflect these changes. My review focuses on improving the robustness and efficiency of the new scripts and fixing a bug in the speakers block implementation. Overall, the changes are extensive and add significant new functionality.
| } | ||
| } catch (error) { | ||
| // Fallback to default behavior | ||
| processTableBasedSpeakers(null, container); |
There was a problem hiding this comment.
This fallback logic will cause a runtime error. The processTableBasedSpeakers function expects a DOM element as its first argument and will attempt to access its children property. Calling it with null will result in a TypeError: Cannot read properties of null (reading 'children'). The catch block should instead log the error and gracefully fail, as there is no table-based content to fall back to in this code path.
| processTableBasedSpeakers(null, container); | |
| console.error(`Failed to load speakers from fragment ${fragmentPath}:`, error); |
| name=$(printf '%s\n' "$frontmatter" | awk -F': *' '/^name:/ {sub(/^name: */,"",$0); print substr($0, index($0,$2))}' 2>/dev/null) | ||
| description=$(printf '%s\n' "$frontmatter" | awk -F': *' '/^description:/ {sub(/^description: */,"",$0); print substr($0, index($0,$2))}' 2>/dev/null) |
There was a problem hiding this comment.
These awk commands for parsing the YAML frontmatter are complex and potentially fragile. For example, they might fail if the name or description values themselves contain colons. Also, redirecting stderr to /dev/null (2>/dev/null) hides potential parsing errors, making debugging harder. For more robust parsing, consider a tool that understands YAML, or simplify the awk script to be more resilient and to not suppress errors.
| process_skills_directory() { | ||
| local skills_dir="$1" | ||
| local location_label="$2" | ||
|
|
||
| if [[ ! -d "$skills_dir" ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| local count=0 | ||
| # Count skills first | ||
| while IFS= read -r -d '' skill_file; do | ||
| count=$((count + 1)) | ||
| done < <(find "$skills_dir" -type f -name 'SKILL.md' -print0) | ||
|
|
||
| if [[ $count -eq 0 ]]; then | ||
| return 0 | ||
| fi | ||
|
|
||
| echo "$location_label ($count skill(s)):" | ||
| # Generate underline matching label length | ||
| local len=${#location_label} | ||
| if [[ $len -gt 0 ]]; then | ||
| local underline="" | ||
| for ((i=0; i<len; i++)); do | ||
| underline+="=" | ||
| done | ||
| echo "$underline" | ||
| fi | ||
| echo "" | ||
|
|
||
| # Iterate SKILL.md files robustly (handles spaces) | ||
| while IFS= read -r -d '' skill_file; do | ||
| skill_dir=$(dirname "$skill_file") | ||
| skill_name=$(basename "$skill_dir") | ||
|
|
||
| # Check for YAML frontmatter | ||
| if head -n 1 "$skill_file" | grep -q "^---$"; then | ||
| # Extract lines between first pair of --- delimiters | ||
| frontmatter=$(awk 'BEGIN{inside=0; c=0} /^---$/ {inside=!inside; if(++c==3) exit} inside==1 {print}' "$skill_file") | ||
| name=$(printf '%s\n' "$frontmatter" | awk -F': *' '/^name:/ {sub(/^name: */,"",$0); print substr($0, index($0,$2))}' 2>/dev/null) | ||
| description=$(printf '%s\n' "$frontmatter" | awk -F': *' '/^description:/ {sub(/^description: */,"",$0); print substr($0, index($0,$2))}' 2>/dev/null) | ||
|
|
||
| echo "Skill: ${name:-$skill_name}" | ||
| echo "Path: $skill_file" | ||
| if [[ -n "$description" ]]; then | ||
| echo "Description: $description" | ||
| fi | ||
| else | ||
| echo "Skill: $skill_name" | ||
| echo "Path: $skill_file" | ||
| echo "Description:" | ||
| head -n 5 "$skill_file" | ||
| fi | ||
|
|
||
| echo "" | ||
| echo "---" | ||
| echo "" | ||
| done < <(find "$skills_dir" -type f -name 'SKILL.md' -print0) | ||
| } |
There was a problem hiding this comment.
This function executes the find command twice with the same arguments, which is inefficient. First to count the files, and second to process them. You can refactor this to run find only once, store the results in an array, and then process the array. This makes the script faster, especially if there are many files to search. The suggested implementation also uses a more idiomatic way to generate the underline.
process_skills_directory() {
local skills_dir="$1"
local location_label="$2"
if [[ ! -d "$skills_dir" ]]; then
return 0
fi
local skill_files=()
while IFS= read -r -d '' file; do
skill_files+=("$file")
done < <(find "$skills_dir" -type f -name 'SKILL.md' -print0)
local count=${#skill_files[@]}
if [[ $count -eq 0 ]]; then
return 0
fi
echo "$location_label ($count skill(s))"
# Generate underline matching label length
local len=${#location_label}
if [[ $len -gt 0 ]]; then
printf '=%.0s' $(seq 1 $len)
echo
fi
echo ""
# Iterate SKILL.md files robustly (handles spaces)
for skill_file in "${skill_files[@]}"; do
skill_dir=$(dirname "$skill_file")
skill_name=$(basename "$skill_dir")
# Check for YAML frontmatter
if head -n 1 "$skill_file" | grep -q "^---$"; then
# Extract lines between first pair of --- delimiters
frontmatter=$(awk 'BEGIN{inside=0; c=0} /^---$/ {inside=!inside; if(++c==3) exit} inside==1 {print}' "$skill_file")
name=$(printf '%s\n' "$frontmatter" | awk -F': *' '/^name:/ {sub(/^name: */,"",$0); print substr($0, index($0,$2))}' 2>/dev/null)
description=$(printf '%s\n' "$frontmatter" | awk -F': *' '/^description:/ {sub(/^description: */,"",$0); print substr($0, index($0,$2))}' 2>/dev/null)
echo "Skill: ${name:-$skill_name}"
echo "Path: $skill_file"
if [[ -n "$description" ]]; then
echo "Description: $description"
fi
else
echo "Skill: $skill_name"
echo "Path: $skill_file"
echo "Description:"
head -n 5 "$skill_file"
fi
echo ""
echo "---"
echo ""
done
}
| const approvedEntries = allEntries.filter(entry => | ||
| entry.approved === 'Yes' || entry.approved === true || entry.approved === 'true' | ||
| ); |
There was a problem hiding this comment.
The check for the approved status is a bit fragile as it checks for multiple specific values ('Yes', true, 'true'). This can be made more robust by converting the value to a lowercase string and checking against 'yes' and 'true'. This handles variations in capitalization and data type more gracefully.
| const approvedEntries = allEntries.filter(entry => | |
| entry.approved === 'Yes' || entry.approved === true || entry.approved === 'true' | |
| ); | |
| const approvedEntries = allEntries.filter(entry => { | |
| const approved = String(entry.approved || '').toLowerCase(); | |
| return approved === 'yes' || approved === 'true'; | |
| }); |
1 participant
Please always provide the GitHub issue(s) your PR is for, as well as test URLs where your change can be observed (before and after):
Fix #
Test URLs: