Conversation
removed duplicate code between vcs.colors and genutil.colors made color utilities function available from vcs.utils and vcs.colors
|
@aashish24 @chaosphere2112 @sankhesh this 20mn PR is finally ready for review. I still need help from @sankhesh for the meshfill animation which I think I broke when merging master in. |
|
@doutriaux1 I'll look into it today. Are we allowed to commit changes today? |
|
@sankhesh Yeah, email out from @aashish24 on Friday indicated freeze was 11:59PM tonight. |
|
Yes, but which timezone? 😜 |
|
@jbeezley Let's shoot for UTC-11 😉 |
In the email I referred to 11:59 PM PT |
|
As per our last meeting, the code freeze will happen on Monday night 11:59 PT. Please try to look at the bugs assigned to you for release 2.4. If for some reason, you think you cannot get to resolve them in time, please contact Dean, Charles, or myself. |
|
its common to have 20mn PR become 2hr PR 😃 |
|
I know but this one went into a 2 weeks PR... |
There was a problem hiding this comment.
Legacy support. All of the old usages of this function would have generated opaque colors, no reason to change that behavior.
|
@doutriaux1 at a high level, the changes makes sense to me. Only had one question but that's because of me not knowing certain things. |
|
LGTM 👍 once we fix the conflict |
Conflicts: Packages/vcs/Lib/utils.py
|
@doutriaux1 is going to fix the merge conflict.. |
|
@doutriaux1 new changes looks reasonable to me. I am going to wait for the buildbot. |
|
@doutriaux1 @sankhesh Looks like the test is still failing (animated meshfill). What I am missing? |
|
@doutriaux1 @sankhesh I figured it out. Atleast one of the problem is missing baseline. I am adding it now |
For testing, we need background (no window) to be true.
|
@sankhesh @doutriaux1 @williams13 I fixed the animation test (really tiny fix). Not sure about the pattern failing test. @sankhesh can you look into it? |
|
@aashish24 that's why when I looked at the meshfill test bg was set to 1 (should have been |
|
mac seem to be broken again, I wonder if the lab scans re |
|
@doutriaux1 test patterns is also failing. Earlier 4 tests were failing but now only three are failing on garant. Do you still want me to merge it? |
There was a problem hiding this comment.
We allow the user to set a null value? Is that intentional?
There was a problem hiding this comment.
Yes, I think for missing value color None used to mean transparent
There was a problem hiding this comment.
Okay. But that shouldn't be the case anymore, right? Since we have an opacity field now.
|
@doutriaux1 👍 LGTM I'm looking at merging my two other branches. |
|
@sankhesh do you think the tests failing are not because of this branch? |
|
The tests are definitely failing because of this branch. But the failures require updating the baselines.
|
|
@sankhesh I think |
|
@doutriaux1 new baseline? |
|
@aashish24 it should all be ready now, let's wait for the bots. |
|
Sure @doutriaux1 |
|
@aashish24 I tihnk it's ready to merge. |
|
@aashish24 crunchy (RH6 FULL) died so it will never complete |
|
@doutriaux1 @aashish24 Tests all pass for me and the bots look good; I'm merging. |
6 participants
@aashish24 @sankhesh I'm still teaking a few things, but somehow when merging master back in, meshfill animations test is now broken @sankhesh do you mind taking a look at this while I fine tune this PR? Thanks.