Skip to content

Remove timestamps and duplicate url from templates - #161

Merged
marianunez merged 2 commits into
mattermost:masterfrom
fmunshi:GH-109-remove-timestamp-messages
Dec 16, 2019
Merged

Remove timestamps and duplicate url from templates#161
marianunez merged 2 commits into
mattermost:masterfrom
fmunshi:GH-109-remove-timestamp-messages

Conversation

@fmunshi

@fmunshi fmunshi commented Nov 29, 2019

Copy link
Copy Markdown
Contributor

Summary

  • Removes timestamps from posts since the mattermost app already shows a timestamp when message is posted.
  • Since the time shown by the github plugin is always UTC it is confusing since it is in a different timezone
  • The consensus from this issue here Make the timezone configurable #109 seemed to be to remove the timestamp entirely

Fixes #109

Screenshots

Heres what it currently looks like
Screen Shot 2019-11-29 at 3 09 38 PM

Heres what the proposed changes look like
Screen Shot 2019-11-29 at 3 09 57 PM

@hanzei
hanzei requested review from levb and marianunez November 30, 2019 07:19
@hanzei hanzei added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester labels Nov 30, 2019
@hanzei hanzei added this to the v0.13.0 milestone Nov 30, 2019
@levb
levb requested a review from aaronrothschild December 1, 2019 13:16
@levb levb added the 1: PM Review Requires review by a product manager label Dec 1, 2019
@levb

levb commented Dec 1, 2019

Copy link
Copy Markdown
Contributor

OT: 0/5 @aaronrothschild @jasonblais with the date stamp removed, should the link and subject be consolidated into a single line, to save space? (or another way to save a line?)

@marianunez marianunez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you for your contribution! 🎉

@levb levb removed the 2: Dev Review Requires review by a core committer label Dec 2, 2019
@levb
levb requested a review from DHaussermann December 2, 2019 17:09
@levb levb self-assigned this Dec 2, 2019

@DHaussermann DHaussermann left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tested and passed.
Ensured that the timestamp is no longer included for delivery of this event.
No issues found
LGTM!

@DHaussermann DHaussermann removed the 3: QA Review Requires review by a QA tester label Dec 3, 2019
@mattermod

Copy link
Copy Markdown
Contributor

This issue has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @jasonblais @hanzei

@jasonblais

Copy link
Copy Markdown
Contributor

We could go as far as combine the first three lines into one via

#new-pull-request [pull-request-title](pull-request-link) by @pull-request-author

image

I am completely 0/5 on what's optimal, though. Worth a separate UX design ticket?

cc @aaronrothschild quick reminder with PM review :)

@aaronrothschild aaronrothschild removed the 1: PM Review Requires review by a product manager label Dec 16, 2019

@aaronrothschild aaronrothschild left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM

@hanzei hanzei added 4: Reviews Complete All reviewers have approved the pull request and removed Lifecycle/1:stale labels Dec 16, 2019
@marianunez
marianunez merged commit 7266f2c into mattermost:master Dec 16, 2019
@hanzei hanzei unassigned levb Dec 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

4: Reviews Complete All reviewers have approved the pull request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make the timezone configurable

8 participants