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

Revert "Added File Menu element to viz.ui" #1577

Closed
wants to merge 1 commit into
base: master
from

Conversation

4 participants
@arokem
Copy link
Member

arokem commented Jun 23, 2018

So sorry! I seem to have merged the wrong PR

@karandeepSJ

This comment has been minimized.

Copy link
Contributor

karandeepSJ commented Jun 24, 2018

@ranveeraggarwal @dmreagan @Garyfallidis Do we need to revert this or is the FileMenu good?

@dmreagan dmreagan added the gsoc2018 label Jun 26, 2018

@dmreagan dmreagan added this to PR needs a review in Viz Module Jun 26, 2018

@dmreagan

This comment has been minimized.

Copy link
Contributor

dmreagan commented Jul 4, 2018

I'd like to merge this to revert #1570, but it looks like it is failing a test because it doesn't include the bugfix from #1574. What's the correct way to proceed?

@karandeepSJ

This comment has been minimized.

Copy link
Contributor

karandeepSJ commented Jul 4, 2018

They failed because this PR was made before the fix was merged. Is there any way of running travis again?

@dmreagan

This comment has been minimized.

Copy link
Contributor

dmreagan commented Jul 11, 2018

We just restarted the failing Travis machine, but I'm guessing it will fail again. I think we need @arokem to rebase this, but keep the two lines from the bugfix in #1574.

@dmreagan

This comment has been minimized.

Copy link
Contributor

dmreagan commented Jul 18, 2018

Can you take a look at this, @arokem?

@arokem arokem force-pushed the revert-1570-FileMenu branch from 40513fc to 7e804bd Jul 18, 2018

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jul 18, 2018

Sorry about the delay here. Rebased!

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 18, 2018

Codecov Report

Merging #1577 into master will decrease coverage by 9.42%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1577      +/-   ##
=========================================
- Coverage   87.32%   77.9%   -9.43%     
=========================================
  Files         246     246              
  Lines       31936   31781     -155     
  Branches     3471    3449      -22     
=========================================
- Hits        27889   24759    -3130     
- Misses       3220    6360    +3140     
+ Partials      827     662     -165
Impacted Files Coverage Δ
dipy/viz/tests/test_ui.py 16.26% <ø> (-67.35%) ⬇️
dipy/viz/ui.py 20.62% <0%> (-68.33%) ⬇️
dipy/viz/tests/test_window.py 12% <0%> (-81.6%) ⬇️
dipy/viz/widget.py 6.71% <0%> (-81.35%) ⬇️
dipy/viz/interactor.py 19.01% <0%> (-79.15%) ⬇️
dipy/viz/actor.py 4.53% <0%> (-78.19%) ⬇️
dipy/viz/tests/test_interactor.py 18.39% <0%> (-71.27%) ⬇️
dipy/viz/colormap.py 17.85% <0%> (-68.58%) ⬇️
dipy/viz/tests/test_actors.py 8.91% <0%> (-64.76%) ⬇️
... and 22 more

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 9994c2a...7e804bd. Read the comment docs.

@arokem arokem referenced this pull request Jul 19, 2018

Merged

Revert 1570 file menu #1590

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jul 19, 2018

Closed in favor of #1590. Hopefully that should pass now, I just implemented the fix from #1574 there as well, and that seems to have been the failing test here.

@arokem arokem closed this Jul 19, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment