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

[cosmetics] get_impacted_account_visitor defined in impacted.cpp and db_notify.cpp #845

Closed
xeroc opened this issue Apr 13, 2018 · 9 comments
Assignees
Labels
2f Testing Status indicating the solution is being evaluated against the test case(s) 4a Low Priority Priority indicating minimal impact to system/user -OR- an inexpensive workaround exists code cleanup low priority

Comments

@xeroc
Copy link
Member

xeroc commented Apr 13, 2018

see title

@oxarbitrage
Copy link
Member

much more harder for me than it should, i tried twice already to get the duplicated code out of impacted and failed.

@abitmore
Copy link
Member

@oxarbitrage afaik @xiangxn has made quite some progress about this issue, although I'm not sure when / whether he will submit a PR. If it's too time-consuming for you (and to avoid potential duplicate efforts), please stop working on this so far.

@oxarbitrage
Copy link
Member

thank you @abitmore , will leave it for @xiangxn. hope to see some progress on it :)

@abitmore abitmore self-assigned this May 15, 2018
@abitmore abitmore added this to the Future Non-Consensus-Changing Release milestone May 15, 2018
@ryanRfox ryanRfox added 2d Developing Status indicating currently designing and developing a solution 4a Low Priority Priority indicating minimal impact to system/user -OR- an inexpensive workaround exists labels May 24, 2018
@xiangxn
Copy link

xiangxn commented May 24, 2018

Yes, I have successfully merged, but there are some differences between the bts code and our project. I need to review the code to determine whether it can be implemented on bts and it is expected to take about 16 hours.

@xiangxn
Copy link

xiangxn commented Jun 14, 2018

I've modified the code and I'm doing some tests now.

@ryanRfox ryanRfox added 2f Testing Status indicating the solution is being evaluated against the test case(s) and removed 2d Developing Status indicating currently designing and developing a solution labels Jun 14, 2018
@ryanRfox ryanRfox self-assigned this Jun 14, 2018
@xiangxn
Copy link

xiangxn commented Jun 15, 2018

This issue conflicts too much with #1055 and needs to wait for #1055 to complete before submitting PR

@ryanRfox
Copy link
Contributor

@xiangxn @abitmore just merged #1055 so will ask for an update on intentions for #845 going forward. Thanks

@xiangxn
Copy link

xiangxn commented Jun 21, 2018

@ryanRfox The code for this issue has been submitted

@oxarbitrage
Copy link
Member

thanks @xiangxn solution was merged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2f Testing Status indicating the solution is being evaluated against the test case(s) 4a Low Priority Priority indicating minimal impact to system/user -OR- an inexpensive workaround exists code cleanup low priority
Projects
None yet
Development

No branches or pull requests

5 participants