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

Fixing code generation bug with underscores in names #208

Merged
merged 4 commits into from Jan 15, 2019

Conversation

vpatryshev
Copy link
Contributor

@vpatryshev vpatryshev commented Jan 13, 2019

Related issues
see issue 205

Describe the proposed solution
Since avro converts names with underscores to CamelCase, we need to refer those CamelCase accessors as generated by avro. Test and test data added.

Describe alternatives you've considered
Could probably tweak avro code generation logic, but why.
We could also use CamelCase in our code, but this seems more confusing.

Additional context
Also, gradlew was tweaked to care about the cases when your jvm is not 8.0, and deprecated flags are not being used anymore.

…score in names.

Avro converts them to CamelCase; so we have to do the same.
A test case added, and a couple of test files.

Also, slightly improved error reporting in command line.

Also, gradlew updated to work correctly on newer versions of jvm.
@salesforce-cla
Copy link

Thanks for the contribution! Before we can merge this, we need @vpatryshev to sign the Salesforce.com Contributor License Agreement.

@codecov
Copy link

codecov bot commented Jan 13, 2019

Codecov Report

Merging #208 into master will decrease coverage by 18.17%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff             @@
##           master     #208       +/-   ##
===========================================
- Coverage   86.36%   68.19%   -18.18%     
===========================================
  Files         310      310               
  Lines       10132    10136        +4     
  Branches      353      548      +195     
===========================================
- Hits         8751     6912     -1839     
- Misses       1381     3224     +1843
Impacted Files Coverage Δ
...in/scala/com/salesforce/op/cli/CommandParser.scala 0% <0%> (-98.08%) ⬇️
...cala/com/salesforce/op/cli/gen/ProblemSchema.scala 0% <0%> (-96.5%) ⬇️
...src/main/scala/com/salesforce/op/cli/CliExec.scala 0% <0%> (-77.78%) ⬇️
...ce/op/stages/impl/feature/TextLenTransformer.scala 0% <0%> (-100%) ⬇️
...lesforce/op/stages/impl/feature/LangDetector.scala 0% <0%> (-100%) ⬇️
...alesforce/op/cli/gen/templates/SimpleProject.scala 0% <0%> (-100%) ⬇️
...main/scala/com/salesforce/op/filters/Summary.scala 0% <0%> (-100%) ⬇️
.../scala/com/salesforce/op/cli/gen/ProblemKind.scala 0% <0%> (-100%) ⬇️
...orce/op/stages/impl/feature/RealNNVectorizer.scala 0% <0%> (-100%) ⬇️
...cala/com/salesforce/op/cli/gen/FileInProject.scala 0% <0%> (-100%) ⬇️
... and 56 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 fd4c42c...976721e. Read the comment docs.

@codecov
Copy link

codecov bot commented Jan 13, 2019

Codecov Report

Merging #208 into master will decrease coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #208      +/-   ##
==========================================
- Coverage   86.36%   86.34%   -0.03%     
==========================================
  Files         310      310              
  Lines       10132    10136       +4     
  Branches      353      548     +195     
==========================================
+ Hits         8751     8752       +1     
- Misses       1381     1384       +3
Impacted Files Coverage Δ
...in/scala/com/salesforce/op/cli/CommandParser.scala 98.11% <100%> (+0.03%) ⬆️
...cala/com/salesforce/op/cli/gen/ProblemSchema.scala 96.55% <100%> (+0.06%) ⬆️
...src/main/scala/com/salesforce/op/cli/CliExec.scala 80% <100%> (+2.22%) ⬆️
.../salesforce/op/features/FeatureBuilderMacros.scala 0% <0%> (-100%) ⬇️

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 fd4c42c...b4ee56c. Read the comment docs.

Copy link
Collaborator

@tovbinm tovbinm left a comment

Choose a reason for hiding this comment

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

lgtm, one minor comment on pwd

@vpatryshev
Copy link
Contributor Author

vpatryshev commented Jan 14, 2019 via email

@vpatryshev
Copy link
Contributor Author

Can we merge it?

@tovbinm tovbinm merged commit d2b195b into salesforce:master Jan 15, 2019
@Jauntbox Jauntbox mentioned this pull request Feb 8, 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.

None yet

2 participants