Skip to content

AIRFLOW-1445: changing HivePartitionSensor UI color to lighter shade#2476

Closed
Acehaidrey wants to merge 1 commit intoapache:masterfrom
Acehaidrey:master
Closed

AIRFLOW-1445: changing HivePartitionSensor UI color to lighter shade#2476
Acehaidrey wants to merge 1 commit intoapache:masterfrom
Acehaidrey:master

Conversation

@Acehaidrey
Copy link
Contributor

@Acehaidrey Acehaidrey commented Jul 24, 2017

Dear Airflow maintainers,

Please accept this PR. I understand that it will not be reviewed until I have checked off all the steps below!

JIRA

  • My PR addresses the following Airflow-1445 issues and references them in the PR title.

Description

  • Here are some details about my PR, including screenshots of any UI changes:

My PR is simply to improve the readability of the text using the HivePartitionSensor. The screen shots below show the before and after. The darker shade (nearly black) is the before, and the purple color is the after.
After -- First suggestion (not being done but second suggestion is)
screen shot 2017-07-23 at 6 19 24 pm

After -- Second suggestion
screen shot 2017-07-29 at 1 55 10 am

Before
screen shot 2017-07-23 at 6 30 06 pm

Tests

  • My PR adds the following unit tests OR does not need testing for this extremely good reason:
    There is no need for testing because it's simply an aesthetic improvement and doesn't affect functionality.

Commits

  • My commits all reference JIRA issues in their subject lines, and I have squashed multiple commits if they address the same issue.

@Acehaidrey
Copy link
Contributor Author

Acehaidrey commented Jul 25, 2017

Does anyone know what the backend tests in travis would fail for this? How can I rerun the test without making a new PR? @allisonwang I'm tagging you because I see you're on the maintainers team and have been active in PRs recently.. Hope that's okay

@codecov-io
Copy link

codecov-io commented Jul 28, 2017

Codecov Report

Merging #2476 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #2476   +/-   ##
======================================
  Coverage    69.5%   69.5%           
======================================
  Files         146     146           
  Lines       11391   11391           
======================================
  Hits         7917    7917           
  Misses       3474    3474
Impacted Files Coverage Δ
airflow/operators/sensors.py 67.36% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 547f818...3b1f9a5. Read the comment docs.

@wolfier
Copy link
Contributor

wolfier commented Jul 28, 2017

Yeah I've been having trouble seeing the names too due to the color. This would be a good improvement!

@allisonwang
Copy link
Contributor

allisonwang commented Jul 29, 2017

@Acehaidrey Wow thanks for making this change! I was also planning to fix it. The color could be even a bit lighter to make the text more clear :)

@Acehaidrey
Copy link
Contributor Author

Acehaidrey commented Jul 29, 2017

Hey @allisonwang thanks for the respond and glad you also noticed this :). You're right I will make it a bit lighter. I've added an image of this below too, to have a lighter shade of the indigo I had originally.
screen shot 2017-07-29 at 1 55 10 am

@allisonwang
Copy link
Contributor

LGTM This looks much better. @aoen would you mind merging this change? thanks!

@aoen
Copy link
Contributor

aoen commented Aug 1, 2017

LGTM

@asfgit asfgit closed this in 836f289 Aug 1, 2017
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