Skip to content

Conversation

SuWebOnes
Copy link

@SuWebOnes SuWebOnes commented Jul 23, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

@SuWebOnes SuWebOnes added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Jul 26, 2025
@LonMcGregor LonMcGregor added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 30, 2025
Copy link

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

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

Hi, good work on this sprint.

  1. Typically we would only commit package files if we are making absolutely necessary changes to the node configuration. Is that the case here?
  2. Good work on the alarm clock, I can see some areas where you could improve it a bit and have left comments there.

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels Jul 30, 2025
…lidation and Added comments to code For clarity and maintainability in alarmclock.js
@SuWebOnes SuWebOnes added 📅 Sprint 3 Assigned during Sprint 3 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Jul 31, 2025
@LonMcGregor
Copy link

Great work on this. Your app is functioning well now. I still see the package files in this commit - do they need to be there? If you didn't mean to change them you don't need to commit them. Do you know how to remove them?

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Jul 31, 2025
@SuWebOnes
Copy link
Author

can i remove them my project folder if it resolves?

@SuWebOnes
Copy link
Author

can you guide me to resolve " Great work on this. Your app is functioning well now. I still see the package files in this commit - do they need to be there? If you didn't mean to change them you don't need to commit them. Do you know how to remove them?"

@LonMcGregor
Copy link

I suggest looking online for help with how to revert files. When you find out how, revert the changes to the package.json files and make a new commit on the branch - this will return them to their original state, and will clean up the commit. It is good practice to only commit files relevant to the feature being worked on, in this case the alarm clock.

@SuWebOnes
Copy link
Author

Hello good evening, Dear reviewer would you mind if you finalize my code, all 'merge conflicts' are resolved

@LonMcGregor
Copy link

If you look at the files tab here on github, you can see that the package.json files and the package-lock.json files are still present. Only files that you are working on during the current sprint / feature should be included, and in this case that doesn't include the node package files.

When you are doing git changes and you see merge conflicts, you need to manually check the files and change them to include only what you expect. If you look in your files now, you will see lines like <<<<<<< HEAD - these should never be committed without fixing them.

@LonMcGregor LonMcGregor added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 4, 2025
@SuWebOnes
Copy link
Author

would you mind if you clarify for me please? i resolved the conflict 'git code in <<<<<<< HEAD ' then pull and the test also working ! i don’t understood the problem

@LonMcGregor
Copy link

Hi Surafel

When we are working with different branches on git we only want to commit files that are absolutely necessary for the feature we are working on. In this case, that is some web apps - an alarm clock, a reading list.

Within your dev environment you can see files used by the development environment - called things like package.json and package-lock.json. For the apps you are developing here I don't think you need to make changes to these, so you should not include the changes as commits.

Because you've committed the files and had a merge issue, and there are now errors in the file Sprint-3/package.json - you can spot these because it contains lines like <<<<<< HEAD - and if someone else tried to use this file it could cause problems.

You have also added new files that did not need to be there like Sprint-3/package-lock.json. Again, this could cause conflicts later on if it gets merged in the pull request, so unless you absolutely mean to commit it, the file should not be there.

This is a full list of the files that may be problematic and should be reverted:

  • Sprint-3/alarmclock/package.json
  • Sprint-3/slideshow/package.json
  • Sprint-3/package.json

You can revert a file at the command line. One way is to use git checkout origin/main Sprint-3/package.json to restore that specific file. You can look up help for how to checkout the files on the main branch in your IDE, if that is vs code or any other.

These files could be deleted as I'm not sure they are necessary for the commit:

  • Sprint-3/package-lock.json
  • package-lock.json
  • package.json

If you make these changes, then commit them to your branch, that should help clean up this pull request so the Files tab on github only shows the files absolutely necessary for the stuff you have worked on

@SuWebOnes
Copy link
Author

GOOD evening I Was trying to correct would you mind if you check it is fixed, and if you have an idea how re do it with out affecting other complete ed files?

@SuWebOnes SuWebOnes added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Aug 5, 2025
@LonMcGregor
Copy link

LonMcGregor commented Aug 6, 2025

Looking at the first commit you made (ce05dda) - that looks like the output I would expect to fix this. If you repeat that for the other files, that will revert them.

Using the command I suggested, if you only give it the names of package files, it won't affect your changes made in other files.

If you still need help, you could get help from a mentor in the slack channel, or you could ask for help in your next in class meeting.

@SuWebOnes
Copy link
Author

Good morning @LonMcGregor , know I completed and resolved the merge conflicts

@SuWebOnes SuWebOnes requested a review from LonMcGregor August 8, 2025 00:08
Copy link

@LonMcGregor LonMcGregor left a comment

Choose a reason for hiding this comment

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

Great. Take care in future when picking which files to commit. You are done with this sprint, well done.

@LonMcGregor LonMcGregor added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Aug 9, 2025
@SuWebOnes
Copy link
Author

thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complete Volunteer to add when work is complete and all review comments have been addressed. 🏕 Priority Mandatory This work is expected 📅 Sprint 3 Assigned during Sprint 3 of this module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants