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

Replace assert with require #159

Merged
merged 2 commits into from
Oct 22, 2018
Merged

Replace assert with require #159

merged 2 commits into from
Oct 22, 2018

Conversation

tovbinm
Copy link
Collaborator

@tovbinm tovbinm commented Oct 20, 2018

Related issues
Assertion checks assert can be removed from the compiled byte code if there happen to be a flag -Xdisable-assertions set at compile time.

Describe the proposed solution
Replace assert with require everywhere in main code + update the tests.

Describe alternatives you've considered
N/A

Additional context
https://www.scala-lang.org/api/current/scala/Predef$.html

@tovbinm tovbinm changed the title Replace asserts with require Replace asserts with requires Oct 20, 2018
@tovbinm
Copy link
Collaborator Author

tovbinm commented Oct 20, 2018

@vpatryshev

@codecov
Copy link

codecov bot commented Oct 20, 2018

Codecov Report

Merging #159 into master will increase coverage by 0.01%.
The diff coverage is 96.55%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #159      +/-   ##
==========================================
+ Coverage   85.75%   85.76%   +0.01%     
==========================================
  Files         303      303              
  Lines        9912     9912              
  Branches      337      543     +206     
==========================================
+ Hits         8500     8501       +1     
+ Misses       1412     1411       -1
Impacted Files Coverage Δ
...sforce/op/utils/spark/OpVectorColumnMetadata.scala 75.55% <100%> (ø) ⬆️
.../op/stages/base/sequence/SequenceTransformer.scala 100% <100%> (ø) ⬆️
...e/op/stages/impl/feature/SmartTextVectorizer.scala 97.4% <100%> (ø) ⬆️
...e/op/stages/impl/insights/RecordInsightsCorr.scala 98.24% <100%> (ø) ⬆️
...sforce/op/stages/impl/feature/Transmogrifier.scala 96.74% <100%> (ø) ⬆️
...m/salesforce/op/stages/OpPipelineStageParams.scala 91.17% <100%> (ø) ⬆️
...p/stages/impl/feature/SmartTextMapVectorizer.scala 100% <100%> (ø) ⬆️
...force/op/stages/impl/feature/OpStringIndexer.scala 100% <100%> (ø) ⬆️
...scala/com/salesforce/op/features/FeatureLike.scala 41.93% <100%> (ø) ⬆️
...stages/base/sequence/BinarySequenceEstimator.scala 100% <100%> (ø) ⬆️
... and 9 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 8264265...86063fa. Read the comment docs.

@tovbinm tovbinm changed the title Replace asserts with requires Replace assert with require Oct 20, 2018
@tovbinm tovbinm merged commit 5e762c3 into master Oct 22, 2018
@tovbinm tovbinm deleted the mt/require branch October 22, 2018 21:37
ericwayman pushed a commit that referenced this pull request Feb 8, 2019
@tovbinm tovbinm mentioned this pull request Jul 11, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants