GLASGOW | Jan-26-ITP | Prakash Dcosta | Sprint 3 | Alarm Clock#1114
GLASGOW | Jan-26-ITP | Prakash Dcosta | Sprint 3 | Alarm Clock#1114dcostaprakash wants to merge 3 commits intoCodeYourFuture:mainfrom
Conversation
dcostaprakash
added
the
Needs Review
Luro91
added
Review in progress
Luro91
left a comment
Luro91
left a comment
There was a problem hiding this comment.
How could you improve the clarity of the pull request checklist?
Sprint-3/alarmclock/index.html
Outdated
| <meta name="viewport" content="width=device-width, initial-scale=1.0" /> | ||
| <link rel="stylesheet" href="style.css" /> | ||
| <title>Title here</title> | ||
| <title>AlarmClock</title> |
There was a problem hiding this comment.
Thank you. I made the change.
Sprint-3/alarmclock/alarmclock.js
Outdated
| let timerId; | ||
| function setAlarm() { | ||
| const input = document.getElementById("alarmSet").value; | ||
| timeRemaining = Number(input); |
There was a problem hiding this comment.
What happens when the user enters a negative number or a string value?
There was a problem hiding this comment.
Yes. I see it is accepting negative and null inputs. I have made the change. Thank you for pointing it out
| const input = document.getElementById("alarmSet").value; | ||
| timeRemaining = Number(input); | ||
|
|
||
| clearInterval(timerId); |
There was a problem hiding this comment.
Nice clearing of the interval to ensure only one countdown is running
Luro91
added
Reviewed
dcostaprakash
added
Needs Review
Luro91
left a comment
Luro91
left a comment
There was a problem hiding this comment.
Well done.
A note on commit messages. It's hard to know for me what happens in a commit if the message is not descriptive. A commit should describe what changed in the code. It's also good practice to split commits into logical blocks (for example for each task or each review comment). This makes it easier to understand and additionally the commits could also be undone more easily if needed.
I like this article about commit messages: https://chris.beams.io/git-commit
Luro91
added
Complete
2 participants
Changelist
I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
My changes meet the requirements of the task
I have tested my changes
My changes follow the style guide
my coursework