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.

Sign up for GitHub

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

Jump to bottom

feat: add reviewed type pull request event #858

Merged
merged 1 commit into from
May 6, 2019
Merged

feat: add reviewed type pull request event #858

merged 1 commit into from
May 6, 2019

Conversation

pd4d10
Copy link
Contributor

@pd4d10 pd4d10 commented Dec 26, 2018

Question Response
Version? v1.4.1
Devices tested? iPhone 7...
Bug fix? yes/no
New feature? yes/no
Includes tests? yes/no
All Tests pass? yes/no
Related ticket? #...

Screenshots

Before After
before after

Description

@coveralls
Copy link

coveralls commented Dec 26, 2018 •
edited
Loading

Coverage Status

Coverage decreased (-0.2%) to 48.688% when pulling 1afefd3 on pd4d10:patch-1 into 9af60b2 on gitpoint:master.

@chinesedfan
Copy link
Member

chinesedfan commented Dec 27, 2018 •
edited
Loading

@pd4d10 Nice start. I didn't have a chance to run your codes because of a little busy, but I can share you some comments first.

  • Will reviewed event contain other states like request changes? If so, the green check icon should switch to a red cross icon.
    • Updated: Yes.
  • The check icon seems not in the center, compared with Github's. It's designed on purpose by Octicons.
  • New introduced words should support i18n. Or someone can do it together in Refactor and internationalize IssueEventListItem #795.

@pd4d10
Copy link
Contributor Author

pd4d10 commented Jan 22, 2019

Sorry for the late response. request changes type added, and icon positions also fixed.

i18n support is not added in this PR because different languages mixed seems weird... Is it better to do it together in #795?

@chinesedfan
Copy link
Member

chinesedfan commented Jan 23, 2019 •
edited
Loading

@pd4d10 Good except a typo. Then we can merge this one. Or if you can solve #868, that will be better. Never mind. Don't have pressure.

@chinesedfan chinesedfan merged commit d0243f6 into gitpoint:master May 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@G2Jose G2Jose G2Jose approved these changes

@chinesedfan chinesedfan chinesedfan approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@pd4d10 @coveralls @chinesedfan @G2Jose