Skip to content

solution#3838

Closed
ghost wants to merge 2 commits into
masterfrom
unknown repository
Closed

solution#3838
ghost wants to merge 2 commits into
masterfrom
unknown repository

Conversation

@ghost
Copy link
Copy Markdown

@ghost ghost commented Aug 29, 2025

No description provided.

Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Decision: Request Changes

Your Sum component is implemented very well—clear, concise, and fully meets the requirements. However, the App component currently violates a core requirement and checklist item #1, which is a critical blocker.

Critical issues to fix (blockers):

  • Location: src/App.jsx
    • On the five Sum renderings (likely lines where <Sum a={...} b={...} /> are declared), all instances explicitly pass both a and b. The requirements state you must render:
      1. <Sum a={2} b={3} />
      2. <Sum a={-5} b={5} />
      3. <Sum a={10} /> (omit b so it defaults to 0)
      4. <Sum b={5} /> (omit a so it defaults to 0)
      5. <Sum /> (omit both so both default to 0)
    • Why this is wrong: The task requires demonstrating defaulting behavior by omitting props, not by explicitly passing 0.
    • How to fix: Update the 3rd, 4th, and 5th instances to omit b, omit a, and omit both, respectively, as shown above.

Minor notes (non-blocking):

  • Your single-line JSX formatting looks good and aligns with the checklist.

Once you adjust App.jsx to omit the props as required, you should be fully compliant. Nice work on the Sum component—just a small tweak in App to finish strong!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Important

Fix the issues, commit the changes, and then re-request my review using the Re-request review button2 in the Reviewers tab.

Tip

If you have any comments about this review, please leave a message in the Add a comment section below and mention me with the @mateacademy-ai-mentor tag3.
I will read your message during my next review of your pull request. Please do not reply directly to my comments, as that feature is not supported yet.

Caution

Do not close the pull request until the review is complete. Closing it early can lead to unexpected behavior and disrupt the review process.

Footnotes

  1. Rate AI review example

  2. Re-request AI review example

  3. Add AI review comments example

Comment thread src/App.jsx Outdated
Comment on lines +7 to +9
<Sum a={2} b={3} />
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the requirements and checklist item #1: You must render five Sum components with the following props:

  • a = 2 and b = 3
  • a = -5 and b = 5
  • only a = 10 (b omitted, defaults to 0)
  • only b = 5 (a omitted, defaults to 0)
  • no props at all (both default to 0)

Currently, all Sum components are rendered with both a and b explicitly set, which does not test the defaulting behavior. You should omit the b prop for the third instance, omit the a prop for the fourth, and omit both for the fifth.

Comment thread src/App.jsx Outdated
<Sum a={2} b={3} />
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
<Sum a={0} b={5} />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the requirements and checklist item #1: You must render five Sum components with the following props:

  • a = 2 and b = 3
  • a = -5 and b = 5
  • only a = 10 (b omitted, defaults to 0)
  • only b = 5 (a omitted, defaults to 0)
  • no props at all (both default to 0)

Currently, all Sum components are rendered with both a and b explicitly set, which does not test the defaulting behavior. You should omit the b prop for the third instance, omit the a prop for the fourth, and omit both for the fifth.

Comment thread src/App.jsx Outdated
<Sum a={-5} b={5} />
<Sum a={10} b={0} />
<Sum a={0} b={5} />
<Sum a={0} b={0} />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This violates the requirements and checklist item #1: You must render five Sum components with the following props:

  • a = 2 and b = 3
  • a = -5 and b = 5
  • only a = 10 (b omitted, defaults to 0)
  • only b = 5 (a omitted, defaults to 0)
  • no props at all (both default to 0)

Currently, all Sum components are rendered with both a and b explicitly set, which does not test the defaulting behavior. You should omit the b prop for the third instance, omit the a prop for the fourth, and omit both for the fifth.

@ghost ghost requested a review from mateacademy-ai-mentor August 29, 2025 09:25
Copy link
Copy Markdown

@mateacademy-ai-mentor mateacademy-ai-mentor left a comment

Choose a reason for hiding this comment

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

Review Summary

Decision: APPROVE

Your implementation fully meets the functional and stylistic requirements:

  • Sum correctly accepts a and b, defaults both to 0 when omitted, and renders exactly: Sum of {a} and {b} is {a + b} using a single interpolation in a <p>.
  • Sum is a named export, and App renders all five required usages, including the cases omitting a, b, and both.
  • JSX usage is clean and on one line where appropriate, matching the checklist.

No critical blockers or discrepancies were found. Nice work—clear, idiomatic React with solid use of destructuring defaults and template literals. Keep it up!


✨ Please rate this review in the Platform's notification center1. I will be grateful for your feedback! ✨

Footnotes

  1. Rate AI review example

@ghost ghost closed this by deleting the head repository Mar 13, 2026
This pull request was closed.
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.

1 participant