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

Fix compile errors with the generated source code #615

Merged
merged 4 commits into from May 30, 2019

Conversation

Projects
None yet
4 participants
@noelmarkham
Copy link
Contributor

commented May 29, 2019

What this does?

  • The "Cop" regex was not picking up the new Option[A] types
  • The "fs2" prefix on Stream was causing the macro to generate the wrong code

Checklist

  • Reviewed the diff to look for typos, println and format errors.
  • Updated the docs accordingly.
Fix compile errors with the generated source code
  - The "Cop" regex was not picking up the new Option[A] types
  - The "fs2" prefix on Stream was causing the macro to generate the wrong code
@noelmarkham

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

@fedefernandez I'm wondering if you're able to help with the Travis failure? https://travis-ci.org/higherkindness/mu/jobs/538794629#L1284

I think it's failing because of the fs2. prefix that was added to Skeuomorph, which is going out as part of this release, but I'm not 100% how to debug and fix this one. My original commit fixed one macro where the fs2. prefix wasn't on a String so a pattern match further on was matching a different case. It may be easier to roll back the original fs2. prefix from Skeuomorph, but I don't know the reason for adding it in the first place.

@fedefernandez

This comment has been minimized.

Copy link
Collaborator

commented May 29, 2019

hey @noelmarkham, I guess you're referring to this one:
higherkindness/skeuomorph#84
The idea behind that was to avoid the import, especially painful when we don't have any stream operations and still adding the import, causing the wart plugin throwing warnings or errors at compile time.

Honestly, I don't know from the top of my head why we're getting this compilation error. I'll try to find some time later to have a look, but we can revert my PR and go back with the previous code as the lesser of two evils.

@noelmarkham

This comment has been minimized.

Copy link
Contributor Author

commented May 29, 2019

Nice one, thanks - I'll update Skeuomorph and push out a new release for that

@codecov

This comment has been minimized.

Copy link

commented May 30, 2019

Codecov Report

Merging #615 into master will increase coverage by 0.1%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master     #615     +/-   ##
=========================================
+ Coverage   82.36%   82.46%   +0.1%     
=========================================
  Files          62       62             
  Lines         964      964             
  Branches       12       12             
=========================================
+ Hits          794      795      +1     
+ Misses        170      169      -1
Impacted Files Coverage Δ
...ndness/mu/rpc/idlgen/proto/ProtoSrcGenerator.scala 93.33% <100%> (+6.66%) ⬆️

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 b619638...1192390. Read the comment docs.

@juanpedromoreno
Copy link
Member

left a comment

Thanks @noelmarkham !

@noelmarkham noelmarkham merged commit e562cd0 into master May 30, 2019

4 checks passed

codecov/patch 100% of diff hit (target 82.36%)
Details
codecov/project 82.46% (+0.1%) compared to b619638
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@noelmarkham noelmarkham deleted the fix-0.18.2-deploy-issues branch May 30, 2019

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.