Fix Part of #8668: Add JsDocs to date-time-format file #10814

linnhallonqvist · 2020-09-29T08:40:55Z

Overview

This PR fixes or fixes part of All the Frontend services should be documented with jsdoc. #8668
This PR does the following: Adding JsDocs to the file date-time-format.service.ts.

Essential Checklist

The PR title starts with "Fix #bugnum: ", followed by a short, clear summary of the changes. (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
The linter/Karma presubmit checks have passed locally on your machine.
"Allow edits from maintainers" is checked. (See here for instructions on how to enable it.)
- This lets reviewers restart your CircleCI tests for you.
The PR is made from a branch that's not called "develop".

PR Pointers

Make sure to follow the instructions for making a code change.
Oppiabot will notify you when you don't add a PR_CHANGELOG label. If you are unable to do so, please @-mention a code owner (who will be in the Reviewers list), or ask on Gitter.
For what code owners will expect, see the Code Owner's wiki page.
Make sure your PR follows conventions in the style guide, otherwise this will lead to review delays.
Never force push. If you do, your PR will be closed.

oppiabot · 2020-09-29T23:00:39Z

Assigning @bansalnitish for code owner reviews. Thanks!

linnhallonqvist · 2020-10-05T12:20:20Z

Hi @bansalnitish , have you had time to look at this? Is it a problem that not all ci tests pass? I have trouble understanding why the tests fail.

bansalnitish

Hi @linnhallonqvist, Took a pass!

bansalnitish · 2020-10-06T05:36:00Z

core/templates/services/date-time-format.service.ts

@@ -45,17 +53,32 @@ export class DateTimeFormatService {
      return moment(date).format('MM/DD/YY');
    }
  }
-  // Returns date along with time.
+  /**
+   * This function converts a millisecond date to a human-readable date


Could you please replace human-readable with the exact format like 'MMM D HH:mm A'?

Can you please merge the latest develop with your branch once and see if the tests pass?

I have adressed your comments and merged developed with my branch. Hope it looks better! @bansalnitish

bansalnitish · 2020-10-06T05:37:36Z

core/templates/services/date-time-format.service.ts

-  // local datetime representation has the same year as the current date.
-  // Otherwise, returns the full date (with the year abbreviated).
+/**
+  * This function returns just the time if the local datetime representation


Remove the word just. Also, can you frame it like The function returns X if Y. Else if A, it returns B. Else, it returns C?

bansalnitish · 2020-10-06T05:39:24Z

core/templates/services/date-time-format.service.ts

+  * if the local datetime representation has the same year as the current
+  * date. Otherwise, returns the full date (with the year abbreviated).
+  * @param {number} millisSinceEpoch - milliseconds since Epoch
+  * @returns {string} - The time if the local datetime representation has the


Ditto for this. Also, I would say adding the complete description once in the main function description. Here you could simply say returns a date or something.

We should not add the same description in the return as well as the function definiton.

bansalnitish · 2020-10-06T05:39:40Z

core/templates/services/date-time-format.service.ts

  getLocaleDateTimeHourString(millisSinceEpoch: number): string {
    let date = new Date(millisSinceEpoch);
    return moment(date).format('MMM D HH:mm A');
  }
-  // Returns just the date.
+  /**
+   * This function converts a millisecond date to a human-readable date.


Ditto for the human-readable.

bansalnitish · 2020-10-06T07:45:23Z

Can you please merge the latest develop with your branch once and see if the tests pass?

DubeySandeep · 2020-10-12T23:47:49Z

Hi @linnhallonqvist, Can you please reply to the open comments once its resolved? (Following the wiki guidelines)

Also, make sure to ping the reviewer once it's ready for review!

DubeySandeep · 2020-10-12T23:48:06Z

@srijanreddy98, Can you please review this PR?

linnhallonqvist · 2020-10-13T08:30:42Z

Hello @DubeySandeep ! Done, sorry for missing that. Also I'm not sure what is meant by "submit the review to add all your messages to the main thread"?

srijanreddy98

LGTM! -- thanks @linnhallonqvist.

oppiabot · 2020-10-14T04:15:42Z

Unassigning @srijanreddy98 since they have already approved the PR.

srijanreddy98 · 2020-10-14T07:58:20Z

@linnhallonqvist congrats on getting your first PR merged.

linha255 and others added 3 commits September 28, 2020 15:02

add jsdoc to date-time-format.service.ts

1be7e43

Update add jsdoc to date-time-format.service.js

f413096

Correct errors

490c133

linnhallonqvist requested a review from bansalnitish as a code owner September 29, 2020 08:40

oppiabot bot assigned bansalnitish Sep 29, 2020

bansalnitish reviewed Oct 6, 2020

View reviewed changes

linha255 added 2 commits October 12, 2020 11:34

Address review comments

42d2e14

Merge branch 'develop' into add-jsdoc-date-time-format

96de718

linnhallonqvist requested a review from srijanreddy98 as a code owner October 12, 2020 11:18

srijanreddy98 assigned srijanreddy98 and unassigned bansalnitish Oct 12, 2020

srijanreddy98 approved these changes Oct 14, 2020

View reviewed changes

oppiabot bot unassigned srijanreddy98 Oct 14, 2020

oppiabot bot added the PR: LGTM label Oct 14, 2020

srijanreddy98 merged commit 5229d2e into oppia:develop Oct 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix Part of #8668: Add JsDocs to date-time-format file #10814

Fix Part of #8668: Add JsDocs to date-time-format file #10814

linnhallonqvist commented Sep 29, 2020

oppiabot bot commented Sep 29, 2020

linnhallonqvist commented Oct 5, 2020

bansalnitish left a comment

bansalnitish Oct 6, 2020

linnhallonqvist Oct 12, 2020

linnhallonqvist Oct 13, 2020

bansalnitish Oct 6, 2020

linnhallonqvist Oct 13, 2020

bansalnitish Oct 6, 2020

linnhallonqvist Oct 13, 2020

bansalnitish Oct 6, 2020

linnhallonqvist Oct 13, 2020

bansalnitish commented Oct 6, 2020

DubeySandeep commented Oct 12, 2020

DubeySandeep commented Oct 12, 2020

linnhallonqvist commented Oct 13, 2020

srijanreddy98 left a comment

oppiabot bot commented Oct 14, 2020

srijanreddy98 commented Oct 14, 2020

Fix Part of #8668: Add JsDocs to date-time-format file #10814

Fix Part of #8668: Add JsDocs to date-time-format file #10814

Conversation

linnhallonqvist commented Sep 29, 2020

Overview

Essential Checklist

PR Pointers

oppiabot bot commented Sep 29, 2020

linnhallonqvist commented Oct 5, 2020

bansalnitish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bansalnitish commented Oct 6, 2020

DubeySandeep commented Oct 12, 2020

DubeySandeep commented Oct 12, 2020

linnhallonqvist commented Oct 13, 2020

srijanreddy98 left a comment

Choose a reason for hiding this comment

oppiabot bot commented Oct 14, 2020

srijanreddy98 commented Oct 14, 2020