databrickslabs / splunk-integration

Databricks Add-on for Splunk
https://splunkbase.splunk.com/app/5416/
Other
26 stars 18 forks source link

Functional incrementation #32

Closed KeerthanaK-18 closed 2 years ago

KeerthanaK-18 commented 2 years ago

Functional incrementation:

Updating the docs and unit test cases

KeerthanaK-18 commented 2 years ago

Blockers for merging this PR:

remove jQuery, that is only used for onClick handling: we need the bundled jquery, because it ensures that jquery 3.5.0 is used. If not, depending on the slunk version used, an older version of jquery may be used which will cause failure in Splunk appinspect report. rename couple of variables, so that it's more maintainable by a larger group of people - will do add documentation changes to note that service principal has to be added to Azure Databricks workspace in order for AAD credentials to work. - Already added why do we need .docx & .pdf versions of the doc? it's not scalable. - It was requested by Arun in the previous release. Hence we are using the same formats and updating it. add GitHub Actions yml file to automatically run tests every time there's commit. if tests are not running automatically - it's equal to not having tests at all. - Adding Separate pull request: code coverage is missing for commands. ignoring most of command files from app/bin in tests/.coveragerc is only hiding the problem. please bring coverage to the required level of 80%, including commands (which are the most important bits of logic for this app). - Since we need to get coverage for the files in bin folder, creating a seperate PR for test cases will not be helpfull because the files in master branch now do not have the functional increment changes merged yet.

nfx commented 2 years ago

@KeerthanaK-18

  1. replace $("#retryButton").click( with document.getElementById('retryButton').onClick( and remove jQuery as dependency, as it makes no sense to have jQuery just for click handling.
  2. rename couple of variables, so that it's more maintainable by a larger group of people - will do
  3. add GitHub Actions yml file to automatically run tests every time there's commit. if tests are not running automatically - it's equal to not having tests at all.
  4. remove copyright from generated file (base.html). it's not allowed.

image

codecov[bot] commented 2 years ago

Codecov Report

:exclamation: No coverage uploaded for pull request base (master@a4de7ce). Click here to learn what that means. The diff coverage is n/a.

@@            Coverage Diff            @@
##             master      #32   +/-   ##
=========================================
  Coverage          ?   86.40%           
=========================================
  Files             ?        8           
  Lines             ?      618           
  Branches          ?        0           
=========================================
  Hits              ?      534           
  Misses            ?       84           
  Partials          ?        0           

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 a4de7ce...b16b493. Read the comment docs.