seattlecolleges.github.io_0_122_Add student name to list of students#142
Conversation
Photo not uploaded yet.
There was a problem hiding this comment.
Looks like your append did not get put to the end of the list, do you mind following up on this and fix this?
There was a problem hiding this comment.
@endlesslupita , can you take a look at this? If you update your PR, make sure to resolve this conversation.
|
The format of the .json file looks good. I think you just have to resolve the conflicts in your branch and it should be good. |
bcko
left a comment
There was a problem hiding this comment.
I will approve this PR with the condition that you will resolve the PR comment provided by @SUPER444E
josehorta21
left a comment
There was a problem hiding this comment.
Reviewed the addition of a new student to students.json. The screenshot
in the description shows the entry rendering correctly on localhost, which
is a good sanity check.
A few specific observations after going through Conversation and Files
changed:
-
Unresolved feedback from @SUPER444E (Mar 7). SUPER444E flagged that
the new entry didn't get appended to the end of the list, and
@endlesslupita's commits (Capitalize filename,Add photo and correct filename) addressed naming issues but it isn't clear from the diff
whether the append-location concern was actually resolved. Worth
confirming the new entry sits at the bottom ofsrc/students.jsonin
the latest state, not somewhere in the middle. -
Conversation still open from @bcko (Mar 19). bcko explicitly asked
@endlesslupita to resolve the SUPER444E conversation thread. Since the
conversation is still showing as open and there hasn't been activity
from the author in roughly 3 months, this PR may have stalled. Worth
either a friendly nudge in the conversation or considering whether the
PR should be closed and the work picked up in a fresh PR if
@endlesslupita is no longer active. -
Merge conflicts in
src/students.json. Same root cause as PRs #158
and #166 — multiple contributors are appending to the same file in
parallel. The conflict will need to be resolved against currentmain
before this can move forward, and the longer it sits the harder the
conflict gets as other student entries land. -
Resolves #122 overlap. Issue #122 ("Students Page: Add yourself to
the Students Page") appears to be a shared issue that multiple PRs
(#158, #166, this one) all reference as "Resolves." Strictly, only one
PR can close the issue, and the issue itself is more of a recurring
task than a one-shot. Worth clarifying with a maintainer whether #122
should be a tracking/umbrella issue rather than something each PR
closes individually. -
Screenshot verification is helpful. The localhost screenshot
showing Zulbadam Batdelger and Jocelyn Graf rendering correctly is a
nice touch — that kind of visual confirmation makes review faster,
especially for first-time contributors. Worth keeping this pattern.
Overall the entry itself looks reasonable, but the PR has been inactive
for ~3 months with unresolved review feedback and merge conflicts. Main
blocker is re-engaging the author (or closing/redirecting the work)
before anything else can move forward.
|
Following up on my review from last week. Checking back on the status of
No new blockers from my side — the original entry content was reasonable, |
Summary & Changes 📃
Resolves: Students Page: Add yourself to the Students Page #122
Summary:
Changes: