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

Add color for MLIR #4726

Merged
merged 2 commits into from
Nov 29, 2019
Merged

Add color for MLIR #4726

merged 2 commits into from
Nov 29, 2019

Conversation

jpienaar
Copy link
Contributor

Description

There is currently no color associated with MLIR, I'm changing it to match one of the colors of the logo of the MLIR project.

Checklist:

@pchaigno
Copy link
Contributor

That color is actually conflicting with MQL4's. From the Travis CI logs:

  1) Failure:
TestColorProximity#test_color_proximity [/home/runner/work/linguist/linguist/test/test_color_proximity.rb:21]:
The following 1 languages have failing color thresholds. Please modify the hex color.
- MQL4 (#62A8D6) is too close to ["5EA8DB", "62A8D6"]

Do you have other ideas of colors? If not, we could simply add it to #4506.

@jpienaar
Copy link
Contributor Author

That color is actually conflicting with MQL4's. From the Travis CI logs:

  1) Failure:
TestColorProximity#test_color_proximity [/home/runner/work/linguist/linguist/test/test_color_proximity.rb:21]:
The following 1 languages have failing color thresholds. Please modify the hex color.
- MQL4 (#62A8D6) is too close to ["5EA8DB", "62A8D6"]

Do you have other ideas of colors? If not, we could simply add it to #4506.

The other colors used are #3775E0 (which is too close to MQL5[1] and Slash), #1B466C (I'm not sure I get MLIR (#1B466C) is too close to ["244776", "1B466C"]) and white (which may not work well on white background of GitHub).

Is the closeness a problem if these two languages will rarely (ever?) end up in same repo? I can't dismiss the possibility though. But that may be irrelevant for current policy.

[1] Why does MQL4 and MQL5 have different colors? Given they are using the same tm_scope and same grammar (but different file extensions) it seem unfortunate given the color different requirement. Esp. given different versions of Python or C++ do not have this difference. If MQL4 shared the same color as MQL5 then it would resolve the conflict on the current suggestion. But this is probably an aside.

@lildude
Copy link
Member

lildude commented Nov 28, 2019

Is the closeness a problem if these two languages will rarely (ever?) end up in same repo?

Unfortunately, yes. I'm still waiting on the GitHub design team to get back to me about dropping this proximity enforcement. I prod the issue now and then but it's not really high on their priority list. Until we get the clearance, we're going to have to keep this enforcement in place.

Whilst you won't be able to get the exact colours used by the language repo, a little tweaking should get you close.

@lildude
Copy link
Member

lildude commented Nov 28, 2019

I just did a quick test and #5EC8DB passes and isn't too far off #5EA8DB. You could experiment a bit more to possibly get closer if you want. Unfortunately, blue is really popular so there will almost always need to be a compromise or a lot of testing.

Consider revising in case the previous one becomes available.
@jpienaar
Copy link
Contributor Author

I just did a quick test and #5EC8DB passes and isn't too far off #5EA8DB. You could experiment a bit more to possibly get closer if you want. Unfortunately, blue is really popular so there will almost always need to be a compromise or a lot of testing.

Yes I can imagine, thanks! I'm good with this suggestion.

Is the closeness a problem if these two languages will rarely (ever?) end up in same repo?

Unfortunately, yes. I'm still waiting on the GitHub design team to get back to me about dropping this proximity enforcement. I prod the issue now and then but it's not really high on their priority list. Until we get the clearance, we're going to have to keep this enforcement in place.

SG, that makes sense. I started typing a longer discussion about different approaches, but figured this would be the wrong spot for that :) Is there perhaps a bug I could follow along for this request? This would be for my own curiosity really as I'm happy enough with a distinct color.

@lildude
Copy link
Member

lildude commented Nov 29, 2019

Is there perhaps a bug I could follow along for this request?

Unfortunately, not. All GitHub issues are currently on the private repo so not open to the public.

@lildude lildude merged commit d3ffd3b into github-linguist:master Nov 29, 2019
@github-linguist github-linguist locked as resolved and limited conversation to collaborators Jun 17, 2024
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.

3 participants