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

small cleanup in a few places #2058

Merged
merged 7 commits into from
May 13, 2018
Merged

small cleanup in a few places #2058

merged 7 commits into from
May 13, 2018

Conversation

nmarley
Copy link

@nmarley nmarley commented May 2, 2018

Please see individual commits. Removes unused code, comments, variables, etc.

Let's wait 'til 12.3 is released before any merge, but I've been itching to get this PR created for a while, so gonna go ahead and do that now.

nmarley added 6 commits May 1, 2018 19:07
* Quells warning displayed with `-Wmaybe-uninitialized` flag
This feels like an optimization, something about:

```
if map.count(key)
     return false
else
     insert(pair)
     return true
```

... just feels icky. Plus, `vote.GetMasternodeOutpoint()` is only called
once. The previous version might be slightly more readable, however.
@UdjinM6
Copy link

UdjinM6 commented May 4, 2018

Looks good 👍

I'd say it's safe to merge in 12.3 if you drop d7f876e for now.

@nmarley
Copy link
Author

nmarley commented May 5, 2018

Per our discussion on another recent PR, we'd like to off on any more non-bug-fixes going in to 12.3, but did you have any specific concerns with commit d7f876e? Or is it just the potential for a bug b/c it's a little more complex?

@UdjinM6
Copy link

UdjinM6 commented May 5, 2018

off on any more non-bug-fixes

It's more like no-new-features-or-potential-changes-in-behaviour mode :)

d7f876e looks fine but has a potential to change the behaviour because it touches some actual code unlike others here.

@nmarley
Copy link
Author

nmarley commented May 6, 2018

Yep, makes sense. Might as well get the others in, will revert d7f876e.

@UdjinM6 UdjinM6 added this to the 12.3 milestone May 11, 2018
Copy link

@UdjinM6 UdjinM6 left a comment

Choose a reason for hiding this comment

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

ACK

@UdjinM6 UdjinM6 merged commit 41680f4 into dashpay:develop May 13, 2018
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
* move nStart closer to used and initialize it

* Quells warning displayed with `-Wmaybe-uninitialized` flag

* remove unused mnEntries vectors

* add devnet magic bytes to mininode.py

* remove unused governance-misc.h file

* remove old TODO comments

* replace map count/insert w/emplace in instantx.cpp

This feels like an optimization, something about:

```
if map.count(key)
     return false
else
     insert(pair)
     return true
```

... just feels icky. Plus, `vote.GetMasternodeOutpoint()` is only called
once. The previous version might be slightly more readable, however.

* Revert "replace map count/insert w/emplace in instantx.cpp"

This reverts commit d7f876e.
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