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

DM-12594: Remove daf::base::Citizen #111

Merged
merged 2 commits into from Jul 1, 2019
Merged

DM-12594: Remove daf::base::Citizen #111

merged 2 commits into from Jul 1, 2019

Conversation

parejkoj
Copy link
Contributor

No description provided.

}
{ authPolicy = pexPolicy::Policy::Ptr(new pexPolicy::Policy(filename)); }
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this happening in its own execution block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ha! Look at the diff above: this was just collapsed by clang-format from lines 89-94 in the original.

So, I have no idea why it's a separate block.

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 I vaguely recall either Jim or Pim working on something to to with the scopeGaurd. I think it was related to a ticket Pim had worked on to make daf persistence thread safe, which is what the scope had to be in place. Now that that is removed I think the scope block can be removed too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You may be right about the reasoning, but according to git history, the scopeGuard was added 10 years ago by smm.

58860a4

I'm tempted to just leave it as is, given the imminent removal of daf_persistence. ;-)

@parejkoj parejkoj merged commit 5e4b7ea into master Jul 1, 2019
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

2 participants