Skip to content

Conversation

@Suleman70
Copy link
Member

@Suleman70 Suleman70 commented Mar 25, 2024

This is for issue #1074 - Make the github embed have more accurate colors

What I have Done:

  1. For issues that are "completed", they become purple as it is assumed their PR's have been merged.

Before:
image
After:
image

  1. For issues that are "not planned", they become gray.

Before:
image
After:
image

3.For PR's that are "drafts", they become gray but lighter than "not planned" issues, so that it could be differentiated.

Before:
image
After:
image

  1. For PR's that are "Merged", they become purple.

Before:
image
After:
image

@Suleman70 Suleman70 requested a review from a team as a code owner March 25, 2024 01:10
@Suleman70 Suleman70 self-assigned this Mar 25, 2024
@Suleman70 Suleman70 changed the title Implementing support for more accurate github status embdeed colors Implementing support for more accurate github embed colors Mar 25, 2024
@Zabuzard
Copy link
Member

pictures for the PR description please. before and after. thanks

@SquidXTV
Copy link
Member

Is there no better purple color for completed? It seems too bright for me and doesn't match the color that GitHub uses.

Reference image:
image

@Taz03
Copy link
Member

Taz03 commented Mar 28, 2024

while we're at it, also change other colors to be github themed

@SquidXTV
Copy link
Member

while we're at it, also change other colors to be github themed

What do you mean exactly? The other ones are GitHub themed already, no?

@Suleman70
Copy link
Member Author

Suleman70 commented Mar 28, 2024

I am not sure whether it is possible with GHIssue to determine whether an PR has been merged or not.

For example this is an PR that has been closed and merged:
image
Its "closed" but has no state reason, so i am not sure how to detect whether it was merged

The classes: GHIssue and its inner static class: PullRequest dont have any relevant fields to determine this.

Does anyone have suggestions?

@Suleman70
Copy link
Member Author

while we're at it, also change other colors to be github themed

Is there any particular color that i should change? The green could be darker, but the red and gray looks good.

@SquidXTV
Copy link
Member

I am not sure whether it is possible with GHIssue to determine whether an PR has been merged or not.

For example this is an PR that has been closed and merged: image Its "closed" but has no state reason, so i am not sure how to detect whether it was merged

The fields in GHIssue and its inner static class: PullRequest dont have any relevant fields to determine this.

Does anyone have suggestions?

I might be out of context but we use kohsuke github api for java right?
I found this GHPullRequest#isMerged method:
https://github-api.kohsuke.org/apidocs/org/kohsuke/github/GHPullRequest.html#isMerged()

@Suleman70
Copy link
Member Author

Suleman70 commented Mar 28, 2024

I am not sure whether it is possible with GHIssue to determine whether an PR has been merged or not.
For example this is an PR that has been closed and merged: image Its "closed" but has no state reason, so i am not sure how to detect whether it was merged
The fields in GHIssue and its inner static class: PullRequest dont have any relevant fields to determine this.
Does anyone have suggestions?

I might be out of context but we use kohsuke github api for java right? I found this GHPullRequest#isMerged method: https://github-api.kohsuke.org/apidocs/org/kohsuke/github/GHPullRequest.html#isMerged()

Thank you so much, I will look into this.

@Suleman70
Copy link
Member Author

It should now work for pull requests.

@SquidXTV
Copy link
Member

It should now work for pull requests.

"it should"? did you test it?

@Suleman70
Copy link
Member Author

Suleman70 commented Mar 31, 2024

It should now work for pull requests.

"it should"? did you test it?

Yes. Ill Update the BEFORE and AFTER's with new Info

SquidXTV
SquidXTV previously approved these changes Mar 31, 2024
Copy link
Member

@ankitsmt211 ankitsmt211 left a comment

Choose a reason for hiding this comment

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

looks good, thanks :)

Copy link
Member

@SquidXTV SquidXTV left a comment

Choose a reason for hiding this comment

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

<3

@ankitsmt211 ankitsmt211 merged commit 08d55ca into Together-Java:develop Apr 4, 2024
@ankitsmt211 ankitsmt211 mentioned this pull request Apr 13, 2024
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.

5 participants