Skip to content
This repository has been archived by the owner on Jan 12, 2024. It is now read-only.

issue#9 - print updated dates #10

Merged
merged 4 commits into from
Nov 29, 2017

Conversation

mohit2agrawal
Copy link

No description provided.

Copy link
Contributor

@mcwhittemore mcwhittemore left a comment

Choose a reason for hiding this comment

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

Thanks for this @mohit2agrawal. It's really close. A few small changes and we can merge

bin/cli.js Outdated
issues
.sort((a, b) => {
if (a.repo === b.repo) return a.id - b.id;
if (a.repo === b.repo){
if (a.updated_at === b.updated_at)
Copy link
Contributor

Choose a reason for hiding this comment

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

the name of the variable is updated

bin/cli.js Outdated
lastDate = null;
}
if (lastDate !== iss.updated_at){
console.log(`\n**${iss.updated_at}**\n`); // eslint-disable-line no-console
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently this prints out undefined as the variable is updated rather than updated_at

@mohit2agrawal
Copy link
Author

Hi @mcwhittemore, I have made the changes.

Copy link
Contributor

@mcwhittemore mcwhittemore left a comment

Choose a reason for hiding this comment

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

Mohit,

Sorry. One more thing. This currently adds date and time rather than just date. This means that most issues will have their own header that looks something like **2017-09-12T00:59:34Z**. The linked ticketed requested the date so this should look something like **2017-09-12**. This should be considered both with printing and when comparing the last date to the current one.

Thanks,
Matthew

@mohit2agrawal
Copy link
Author

Hi @mcwhittemore
Please check with the changed date.

@mcwhittemore mcwhittemore merged commit 44da39f into mapbox:master Nov 29, 2017
@mcwhittemore
Copy link
Contributor

Thanks @mohit2agrawal!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants