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

Explain concurrency limitation #972

Merged
merged 2 commits into from
Jan 7, 2020

Conversation

mnm678
Copy link
Contributor

@mnm678 mnm678 commented Jan 4, 2020

Fixes issue #: #49

Description of the changes being introduced by the pull request: Adds a note to the readme explaining that the reference implementation is not thread safe.

Signed-off-by: marinamoore <mmoore32@calpoly.edu>
Copy link
Member

@lukpueh lukpueh left a comment

Choose a reason for hiding this comment

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

LGTM.

Although this (rather technical) piece of information looks a bit lost in this (rather organizational) document. But I don't know a better place. So I'm fine with leaving it here for the time being.

README.md Outdated
-----------

The reference implementation may behave unexpectedly when concurrently
downloading from the same location to the same location.
Copy link
Member

Choose a reason for hiding this comment

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

Sorry, could you clarify what you mean by "location" here? Thanks!

Signed-off-by: marinamoore <mmoore32@calpoly.edu>
@mnm678
Copy link
Contributor Author

mnm678 commented Jan 7, 2020

Thanks for the reviews!

@trishankatdatadog I updated this per your comment

Copy link
Member

@trishankatdatadog trishankatdatadog left a comment

Choose a reason for hiding this comment

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

💯

@mnm678 mnm678 merged commit b913d88 into theupdateframework:develop Jan 7, 2020
@mnm678 mnm678 deleted the thread-safety branch January 7, 2020 23:18
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.

3 participants