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

Imaging browser undef variables and missing prepare array #1869

Merged
merged 2 commits into from
Jun 3, 2016

Conversation

gluneau
Copy link
Contributor

@gluneau gluneau commented Jun 1, 2016

renamed js file and added it to the getJSDependencies, that fixes this warning:

WARNING: Relying on imaging_browser.js getting automatically loaded will soon be removed. Override getJSDependencies() instead

@gluneau gluneau added the Cleanup PR or issue introducing/requiring at least one clean-up operation label Jun 1, 2016
@gluneau gluneau added this to the 16.0 milestone Jun 1, 2016
@driusan
Copy link
Collaborator

driusan commented Jun 1, 2016

I don't have any problems with this change, but I think it should go to 16.1. I don't think it's that urgent to fix a warning which doesn't break anything when we're already at RC2.

@driusan
Copy link
Collaborator

driusan commented Jun 1, 2016

On second thought, I just noticed the fix to the prepared statement, which I think should absolutely go in.. though I'm not sure why they're in the same pull request.

@@ -578,9 +578,10 @@ class NDB_Form_Imaging_Browser extends NDB_Form
$depends = parent::getJSDependencies();
if($this->page === 'viewSession') {
$factory = NDB_Factory::singleton();
$baseURL = $factory->settings()->getBaseURL();
$baseurl = $factory->settings()->getBaseURL();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also this variable was also wrong, if the baseurl has a directory, it would fail without this fix.

@codecov-io
Copy link

codecov-io commented Jun 1, 2016

Current coverage is 14.34%

Merging #1869 into 16.04-dev will increase coverage by <.01%

  1. 2 files (not in diff) in php/libraries were modified. more
    • Misses -7
    • Hits +7
  2. File ..._dashboard.class.inc (not in diff) was modified. more
    • Misses -1
    • Partials 0
    • Hits +1
@@           16.04-dev      #1869   diff @@
===========================================
  Files            118        118          
  Lines          19653      19655     +2   
  Methods         1084       1084          
  Messages           0          0          
  Branches           0          0          
===========================================
+ Hits            2811       2819     +8   
+ Misses         16842      16836     -6   
  Partials           0          0          

Sunburst

Powered by Codecov. Last updated by 789a9a6...1d10b19

@samirdas samirdas merged commit 9377c70 into aces:16.04-dev Jun 3, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Cleanup PR or issue introducing/requiring at least one clean-up operation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants