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 bug descriptors pmgdqh #47

Merged
merged 3 commits into from
Nov 6, 2018
Merged

Conversation

luisremis
Copy link
Contributor

Fix #37 , #33.

@luisremis luisremis requested a review from vishakha041 October 24, 2018 23:49
@luisremis luisremis force-pushed the fix_bug_descriptors_pmgdqh branch 2 times, most recently from 0778031 to 584c749 Compare October 30, 2018 22:20
vishakha041
vishakha041 previously approved these changes Nov 1, 2018
Copy link
Contributor

@vishakha041 vishakha041 left a comment

Choose a reason for hiding this comment

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

Do split the commits where needed and maybe name the branch better? :)

@@ -0,0 +1,256 @@
#
# The MIT License
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you please split out the test commit.

@@ -876,7 +877,7 @@ Json::Value FindDescriptor::construct_responses(

if (cache.isMember("cache_obj_id")) {
// TODO CHECK THIS UNSAFE ERASE
_cache_map.unsafe_erase(cache["cache_obj_id"].asInt64());
// _cache_map.unsafe_erase(cache["cache_obj_id"].asInt64());
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason you have this line commented out? If not needed, just remove it

for i in range(0,concurrency):
thread_add = Thread(target=self.addEntity,args=(i, results) )
thread_add.start()
thread_arr.append(thread_add)
Copy link
Contributor

Choose a reason for hiding this comment

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

What are these set of changes for? I am lazy to figure it out myself :) seems like a different commit from a quick scan.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

stuff for checking that concurrent read/writes worked. you only need to worry about it if the test fails :)

@luisremis luisremis force-pushed the fix_bug_descriptors_pmgdqh branch from 584c749 to ca6523f Compare November 6, 2018 01:55
@luisremis luisremis merged commit efdd05e into develop Nov 6, 2018
@luisremis luisremis deleted the fix_bug_descriptors_pmgdqh branch May 3, 2019 23:16
Ragaad pushed a commit to Ragaad/vdms that referenced this pull request Jun 16, 2022
* Adding FLINNG Library

* Add the default  value of parameters of the Flinng descriptor Set

* Update both dockerfiles to match (other than coverage)

* remove parma value from Flinngvoid FlinngDescriptorSet::getFlinngParams
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.

2 participants