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

Updating orange hex #15

Merged
merged 8 commits into from
Jan 30, 2022
Merged

Updating orange hex #15

merged 8 commits into from
Jan 30, 2022

Conversation

jennank
Copy link
Contributor

@jennank jennank commented Jan 29, 2022

Purpose

The goal of this PR is to resolve issue #14, replacing the current orange with #ff7f0e.

Type of change

  • Bugfix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (non-backwards-compatible fix or feature)
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Documentation update
  • Maintenance update
  • Other (please describe)

Testing

N / A

Checklist

  • I have run flake8 and black to make sure the code adheres to PEP-8 and is consistently formatted
  • I have run unit and regression tests which pass locally with my changes
  • I have added new tests that prove my fix is effective or that my feature works
  • I have added necessary documentation

@jennank jennank requested a review from a team as a code owner January 29, 2022 23:34
@jennank jennank requested a review from A-CGray January 29, 2022 23:34
@jennank
Copy link
Contributor Author

jennank commented Jan 29, 2022

I see the tests fail, these aren't lines I changed but I can run the formatter and update them for this PR.

@A-CGray
Copy link
Member

A-CGray commented Jan 29, 2022

Hey Jenna, thanks for pointing this out, I'd never noticed. I like the new orange but it is now a little close to the yellow/gold colour, although definitely not as close as the previous version was to red (see below). Can we tweak the orange ever so slightly so that it's a bit more distinct?

I would like to keep the old orange as I think it's a nice colour, but I'll rename it RedOrange and also remove it from the colour cycle

Screenshot from 2022-01-29 18-51-44

@jennank
Copy link
Contributor Author

jennank commented Jan 30, 2022

I was unable to reproduce the errors locally, but changed the files based on the GH log. This now directly conflicts with local black, but should pass the checks on GH.

@jennank
Copy link
Contributor Author

jennank commented Jan 30, 2022

It is similar, you're right. We could try making the orange lighter, such as #ff8f00, or changing the yellow to something like #FFD700 to make it more yellow, and less golden.

@A-CGray
Copy link
Member

A-CGray commented Jan 30, 2022

Ugh, I dunno what's happening with the formatting, will fix tomorrow

@ewu63
Copy link
Collaborator

ewu63 commented Jan 30, 2022

Unlike the checks in Azure, the YAML file doesn't specify the version of black used to check the format. Incidentally, they just released a new version today (no longer in beta) that made some changes to the format output. I suggest that we pin the version until the entire mdolab organization is ready to move to the new format. As a side note, they have now promised to do this only once a year in the first release, so it should be more stable going forward.

@A-CGray A-CGray merged commit 76ae041 into mdolab:master Jan 30, 2022
@eirikurj eirikurj mentioned this pull request Jan 30, 2022
@jennank jennank deleted the updatingOrangeHex branch January 30, 2022 15:19
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.

None yet

3 participants