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

Small changes here and there #1799

Merged
merged 3 commits into from Dec 23, 2016

Conversation

Projects
None yet
5 participants
@kernc
Copy link
Member

commented Dec 2, 2016

Random stuff. See individual commits.

The commit that broke WebviewWidget has been moved to #1816.

Includes
  • Code changes
  • Tests
  • Documentation

@kernc kernc referenced this pull request Dec 2, 2016

Merged

[ENH] Map widget #1735

2 of 3 tasks complete
@codecov-io

This comment has been minimized.

Copy link

commented Dec 2, 2016

Current coverage is 89.16% (diff: 100%)

Merging #1799 into master will not change coverage

@@             master      #1799   diff @@
==========================================
  Files            85         85          
  Lines          9058       9058          
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
  Hits           8077       8077          
  Misses          981        981          
  Partials          0          0          

Sunburst

Powered by Codecov. Last update 0572261...5a0287f

kernc added a commit to biolab/orange3-timeseries that referenced this pull request Dec 5, 2016

LineChart, Spiralogram: don't pass QWidget as pybridge
Refs: "WebView/WebEngine: don't permit exposing QWidgets"
	  biolab/orange3#1799

kernc added a commit to kernc/orange3-educational that referenced this pull request Dec 5, 2016

JS widgets: don't bridge via QWidget but via thin QObject instead
Refs: "WebView/WebEngine: don't permit exposing QWidgets"
      biolab/orange3#1799
@janezd

This comment has been minimized.

Copy link
Contributor

commented Dec 5, 2016

The purpose of the PR's description is to explain what the PR is about.

  • Please replace "Fixups" in the title with "[FIX]". Then, provide a title.
  • Change "Random stuff" to something better than foo.
  • Explain what is meant by "One of the commits breaks some WebviewWidget users like add-ons". It's cryptic. What is a "WebviewWidget user"? An actual person or another widget, possibly from an add-on? Does one of these commits do break it? Please say which and how and why and what to do about it.

If the whatever this PR fixes can and should be tested, please add tests.

It would seem that this PR fixes at least three totally unrelated things: dependencies, something related to webview and results of several itemmodels' methods. Should it be broken into multiple PRs?

Please either correct me if I'm wrong or improve this PR according to what meets your criteria for an acceptable PR.

@kernc kernc changed the title Fixups [WIP] Random shit Dec 9, 2016

README.md Outdated
@@ -24,7 +24,8 @@ it in a development environment, run:

# Install some build requirements via your system's package manager
sudo apt-get install virtualenv git python3-dev g++ gfortran \
libblas-dev liblapack-dev libatlas-base-dev
libblas-dev liblapack-dev libatlas-base-dev \
libssl-dev libffi-dev # ...

This comment has been minimized.

Copy link
@astaric

astaric Dec 10, 2016

Member

Which package needs these libraries?

This comment has been minimized.

Copy link
@kernc

kernc Dec 12, 2016

Author Member

No idea, possibly NumPy stack. I had them installed. Anyway, I think these instructions are obsolete. Will update.

@kernc kernc force-pushed the kernc:fixup branch from 2f1ab87 to cc57a76 Dec 12, 2016

@kernc kernc changed the title [WIP] Random shit Small changes here and there Dec 12, 2016

@kernc kernc changed the title Small changes here and there [WIP] Small changes here and there Dec 12, 2016

@kernc

This comment has been minimized.

Copy link
Member Author

commented Dec 12, 2016

@janezd Thanks, you're right. I have batched the most offending commit along with others related into #1816. This now contains three totally unrelated things, any of which by itself seems too light to bear its own whole PR. They're small and unobtrusive changes; kind of something one would just push herself if it weren't for policy. I don't have a problem with that.

@kernc kernc force-pushed the kernc:fixup branch 7 times, most recently from 2c8d0b3 to acaafbd Dec 12, 2016

@kernc kernc changed the title [WIP] Small changes here and there Small changes here and there Dec 13, 2016

@kernc kernc force-pushed the kernc:fixup branch 3 times, most recently from 015e763 to a400e9f Dec 20, 2016

kernc added some commits Dec 2, 2016

@kernc kernc force-pushed the kernc:fixup branch from a400e9f to 5a0287f Dec 20, 2016

@kernc kernc requested a review from ajdapretnar Dec 20, 2016

@ajdapretnar

This comment has been minimized.

Copy link
Contributor

commented Dec 22, 2016

5a0287f is probably what I need to review, but I have no idea how...

@astaric astaric merged commit 8ea73bf into biolab:master Dec 23, 2016

5 checks passed

codecov/patch Coverage not affected when comparing 0572261...5a0287f
Details
codecov/project 89.16% (+0.00%) compared to 0572261
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
licence/cla Contributor License Agreement is signed.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.