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

Task Ref now increments nonce with every set #749

Merged
merged 5 commits into from
Oct 26, 2016

Conversation

AdamChlupacek
Copy link
Contributor

As per comments in fs2.Task.ref the nonce should be updated with every successful set or modify, up till now for set the nonce was updated only in case of the first set, this PR addresses this issue.

Added a test that demonstrates the case in which this would be an issue. This would also affect modify of the ref in case there was a concurrent set on it.


/**
* Created by adamchlupacek on 22/10/16.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update comment to be inline with rest of fs2

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will remove this one all together

@pchlupacek
Copy link
Contributor

@pchiusano @mpilquist If there are no objections can we merge this?


"Task" - {
"Ref" - {
"Set increments nonce" in {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think nonce is here internal implementation detail. What we really solving here is interleaving set and modify// access. I would prefer updated name with perhaps a short comment explaining what this tests verifies.

@mpilquist
Copy link
Member

👍 Per Pavel's last comment, let's update the test name and then we can merge.

@pchiusano
Copy link
Contributor

👍

On Mon, Oct 24, 2016 at 4:20 AM, Pavel Chlupacek notifications@github.com
wrote:

@pchlupacek commented on this pull request.

In core/jvm/src/test/scala/fs2/TaskSpec.scala
#749 (review)
:

@@ -0,0 +1,23 @@
+package fs2
+
+import scala.concurrent.duration._
+
+class TaskSpec extends Fs2Spec{
+

  • "Task" - {
  • "Ref" - {
  •  "Set increments nonce" in {
    

I think nonce is here internal implementation detail. What we really
solving here is interleaving set and modify// access. I would prefer
updated name with perhaps a short comment explaining what this tests
verifies.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#749 (review),
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAArQnTXB9aFfwJ0DUXeThj_veI7Kb6Wks5q3GpbgaJpZM4Kd_ix
.

@mpilquist mpilquist merged commit 7810f49 into typelevel:series/0.9 Oct 26, 2016
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.

4 participants