-
Notifications
You must be signed in to change notification settings - Fork 4k
Date and time #3346
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Open
Open
Date and time #3346
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There is no variable named date prior to this mention. We could keep `.getTime( )` as is, or use the variable `now` or `Jan01_1970`.
According to MDN and some other docs, the term day is used. I did not check the specification, but day would be a more accurate description.
modified by the same logic of line 388
1-js/05-data-types/11-date/7-get-seconds-to-tomorrow/solution.md
Outdated
Show resolved
Hide resolved
```
`new Date(year, month, date, hours, minutes, seconds, ms)`
`new Date(year, month, day, hours, minutes, seconds, ms)`
Contributor
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
would date be more clear? Since new Date().getDay() returns the number of the weekday? 🤔
Co-authored-by: Jerald Vinfrank <46400789+JeraldVin@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Line 34 - There is no variable named date prior to this mention. We could keep
.getTime( )as is, or use the variablenoworJan01_1970.line 57 - Though
dateis used in some docs,daymakes more sense and I also saw day in MDN. P.S this is just modified in one place: if agreed, will have to change this in multiple places.line 61 - grammar fix
line 62 - replaced
is actually the day ofwithrepresents the day of. I swapped date with day ahead of time, we can ignore that if line 57 does not change.line 69 - typo
line 108 - replaced
shiftedwithoffsetline 167 - grammar fix
line 244 - took out
in JavaScriptbecause it is implicit and may improve readability.line 377 - suggestion
line 390 - modified by the same logic(format) of line 388
line 402 - extra space
line 415 - took out
in JavaScriptbecause it is implicit and may improve readability.line 416 - suggestion
line 424 -
returnsmay be more accurateTasks 2 and 3 - formatting improvements
Tasks 7 and 8 - some improvements