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 #333 Mono.whenDelayError with 3 parameters takes only last 2 parameter #334

Merged
merged 6 commits into from Jan 5, 2017

Conversation

sinwe
Copy link
Contributor

@sinwe sinwe commented Jan 3, 2017

Issue is described at #333

@codecov-io
Copy link

codecov-io commented Jan 3, 2017

Current coverage is 60.60% (diff: 100%)

Merging #334 into master will decrease coverage by 0.02%

@@             master       #334   diff @@
==========================================
  Files           261        261          
  Lines         23792      23792          
  Methods           0          0          
  Messages          0          0          
  Branches       4357       4357          
==========================================
- Hits          14424      14419     -5   
- Misses         7527       7532     +5   
  Partials       1841       1841          

Powered by Codecov. Last update bd113a1...b672e50

@simonbasle simonbasle changed the title Fixed https://github.com/reactor/reactor-core/issues/333 Mono.whenDelayError with 3 parameters are only taking the last 2 parameter Jan 4, 2017
@simonbasle
Copy link
Member

simonbasle commented Jan 4, 2017

thanks for that contribution @sinwe 🎁

a couple of things before we can look at merging this:

  • can you confirm that you have signed the Spring CLA?
  • can you amend your commit to avoid changing the imports order? (Note there is a code formatting profile for IntelliJ and Eclipse in the project in the /codequality/ folder)
  • also please try to follow the commit title format of fix #xxx Description in less than 72 chars
  • ideally we're looking at moving away from Groovy based tests. Would you mind rewriting your test in Java inside of the FluxSpecTests class (put it at the beginning)? It was added recently with Migrate first 51 tests of FluxSpec to JUnit #332...

@sinwe
Copy link
Contributor Author

sinwe commented Jan 4, 2017

@simonbasle

  • I signed already, but not sure if it was Spring CLA or the other one... Basically the one used for personal. I'm not representing my company on this.
  • The test is related to Mono, are you sure you want me to put in FluxSpecTests instead of MonoSpecTest (non-existence)

@simonbasle
Copy link
Member

ah good point, ok let's keep it in Groovy then ;)

@sinwe sinwe changed the title Mono.whenDelayError with 3 parameters are only taking the last 2 parameter Fix #333 Mono.whenDelayError with 3 parameters takes only last 2 parameter Jan 4, 2017
@sinwe
Copy link
Contributor Author

sinwe commented Jan 4, 2017

@simonbasle I've used the code formatting for intellij and amend the pull request title

@simonbasle
Copy link
Member

argh somehow it is worse... what I wanted it for the Mono class to only have 1 line changed (the actual fix) in the diff (https://github.com/reactor/reactor-core/pull/334/files)

@sinwe
Copy link
Contributor Author

sinwe commented Jan 4, 2017

That is weird, I was pretty much sure that I only 1 line change.. I guess I messed up with git history. Let me redo this.

@sinwe
Copy link
Contributor Author

sinwe commented Jan 4, 2017

@simonbasle done

@simonbasle simonbasle merged commit 1fed1e7 into reactor:master Jan 5, 2017
@simonbasle
Copy link
Member

Thanks for the PR @sinwe! Merged

simonbasle added a commit that referenced this pull request Jan 9, 2017
simonbasle added a commit that referenced this pull request Jan 10, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants