Skip to content
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

Added tl add command. #116

Merged
merged 7 commits into from
Sep 20, 2019
Merged

Added tl add command. #116

merged 7 commits into from
Sep 20, 2019

Conversation

jibran
Copy link
Contributor

@jibran jibran commented Sep 2, 2019

Sample usage:

tl add 12355 0.5 to log half an hour to ticket 12355.
tl add 12355 1 "Doin stuff" to log an hour to ticket 12355 with comment.
tl add 12355 .25 "Daily Scrum" -s 11am to log time for 11am scrum.
tl add 12345 1 -s 2018-12-31 11 am to log time for specfic date.

Signed-off-by: Jibran Ijaz jibran.ijaz@gmail.com

Sample usage:

`tl add 12355 0.5` to log half an hour to ticket 12355.
`tl add 12355 1 "Doin stuff"` to log an hour to ticket 12355 with comment.
`tl add 12355 .25 "Daily Scrum" -s 11am` to log time for 11am scrum.
`tl add 12345 1 -s 2018-12-31 11 am` to log time for specfic date.

Signed-off-by: Jibran Ijaz <jibran.ijaz@gmail.com>
@Sam152
Copy link
Contributor

Sam152 commented Sep 3, 2019

Out of interest, how does -s 11am work? I can't see start times reported anywhere in Jira?

@dpi
Copy link
Contributor

dpi commented Sep 3, 2019

@Sam152 start time is posted to Jira as the entry time. When you click Work logs tab in Jira it shows when it was posted, in same fashion as comments.

src/Commands/Add.php Outdated Show resolved Hide resolved
src/Commands/Add.php Outdated Show resolved Hide resolved
src/Commands/Add.php Outdated Show resolved Hide resolved
Copy link
Contributor

@dpi dpi left a comment

Choose a reason for hiding this comment

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

Thanks @jibran

Looks what I had in mind. Primarily notes around error and user feedback.

src/Commands/Add.php Outdated Show resolved Hide resolved
$params[':comment'] = $comment;
}
$slot_id = $this->repository->insert($record, $params);
$output->writeln(sprintf('<bg=blue;fg=white;options=bold>[%s]</> Added entry for <info>%d</info>: %s [slot:<comment>%d</comment>] for <info>%s</info>.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for outputting the start time in a consistent fashion since the user may be unaware how PHP interprets human date strings..

I think we should reorder message by importance of information in conversational tone:

.... Added entry for TICKETNUMBER:TICKETNAME on DATE for HOURS (Slot XYZ).

Slot ID is less important for add-command since its not likely user needs to reference it later.

Copy link
Owner

Choose a reason for hiding this comment

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

also, is the bg = blue a new thing? can we keep it consistent with other commands ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

also, is the bg = blue a new thing? can we keep it consistent with other commands ?

It is same as https://github.com/larowlan/tl/blob/master/src/Commands/Start.php#L100

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@dpi let me know if it seems better now.

src/Commands/Add.php Outdated Show resolved Hide resolved
tests/Commands/AddTest.php Outdated Show resolved Hide resolved
tests/Commands/AddTest.php Show resolved Hide resolved
tests/Commands/AddTest.php Show resolved Hide resolved
tests/Commands/AddTest.php Show resolved Hide resolved
@dpi
Copy link
Contributor

dpi commented Sep 3, 2019

Duration arg should be reworked with the stuff from #115, if it lands first.

@larowlan
Copy link
Owner

can you merge master on this one @jibran

Copy link
Owner

@larowlan larowlan left a comment

Choose a reason for hiding this comment

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

Looking good, agree with review comments by dpi

src/Commands/Add.php Outdated Show resolved Hide resolved
$params[':comment'] = $comment;
}
$slot_id = $this->repository->insert($record, $params);
$output->writeln(sprintf('<bg=blue;fg=white;options=bold>[%s]</> Added entry for <info>%d</info>: %s [slot:<comment>%d</comment>] for <info>%s</info>.',
Copy link
Owner

Choose a reason for hiding this comment

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

also, is the bg = blue a new thing? can we keep it consistent with other commands ?

Signed-off-by: Jibran Ijaz <jibran.ijaz@gmail.com>
@jibran jibran merged commit 83e0273 into master Sep 20, 2019
@jibran jibran deleted the add-add-command branch September 20, 2019 01:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants