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

blocks: use auto-generated .c files instead of 'LD -r -b binary' #4536

Merged

Conversation

xiphon
Copy link
Contributor

@xiphon xiphon commented Oct 9, 2018

Rebased #4472 + Fixes OSX compilation

@xiphon xiphon force-pushed the libblocks-auto-generated-c-files branch 2 times, most recently from 4508ada to fb9fb63 Compare October 9, 2018 06:09
@xiphon xiphon changed the title blocks: use auto-generated .c files instead of 'LD -r -b binary' [DO NOT MERGE] blocks: use auto-generated .c files instead of 'LD -r -b binary' Oct 9, 2018
@xiphon xiphon force-pushed the libblocks-auto-generated-c-files branch 7 times, most recently from b58d7b4 to 7db4799 Compare October 9, 2018 12:48
@xiphon xiphon changed the title [DO NOT MERGE] blocks: use auto-generated .c files instead of 'LD -r -b binary' blocks: use auto-generated .c files instead of 'LD -r -b binary' Oct 9, 2018
@xiphon
Copy link
Contributor Author

xiphon commented Oct 9, 2018

Rebased, ready to merge

Copy link
Collaborator

@moneromooo-monero moneromooo-monero left a comment

Choose a reason for hiding this comment

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

Just one minor thing, otherwise seems good to go.

@xiphon
Copy link
Contributor Author

xiphon commented Oct 21, 2018

@moneromooo-monero
which one?

@moneromooo-monero
Copy link
Collaborator

Weird, I'm pretty sure I typed a review comment. It's about making the std::function parameters const refs, as they can get largeish.

@xiphon xiphon force-pushed the libblocks-auto-generated-c-files branch 2 times, most recently from bd04aa7 to 5c12b0c Compare October 21, 2018 20:24
@xiphon
Copy link
Contributor Author

xiphon commented Oct 21, 2018

@moneromooo-monero
Copy link
Collaborator

There's at least a couple which are still not, functions in core and Blockchain.

@xiphon
Copy link
Contributor Author

xiphon commented Oct 21, 2018

Kinda hard to guess what lines we are talking about. Feel free to comment on these lines.

Copy link
Collaborator

@moneromooo-monero moneromooo-monero left a comment

Choose a reason for hiding this comment

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

done

src/cryptonote_core/blockchain.cpp Outdated Show resolved Hide resolved
src/cryptonote_core/blockchain.h Outdated Show resolved Hide resolved
src/cryptonote_core/blockchain.h Outdated Show resolved Hide resolved
src/cryptonote_core/cryptonote_core.cpp Outdated Show resolved Hide resolved
src/cryptonote_core/cryptonote_core.h Outdated Show resolved Hide resolved
@xiphon xiphon force-pushed the libblocks-auto-generated-c-files branch from 5c12b0c to fd62b6e Compare October 21, 2018 22:11
@xiphon
Copy link
Contributor Author

xiphon commented Oct 21, 2018

Thanks, done

Copy link
Collaborator

@moneromooo-monero moneromooo-monero left a comment

Choose a reason for hiding this comment

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

ty

Copy link
Contributor

@fluffypony fluffypony left a comment

Choose a reason for hiding this comment

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

Reviewed

@fluffypony fluffypony merged commit fd62b6e into monero-project:master Oct 26, 2018
fluffypony added a commit that referenced this pull request Oct 26, 2018
fd62b6e blocks: use auto-generated .c files instead of 'LD -r -b binary' (xiphon)
@xiphon xiphon deleted the libblocks-auto-generated-c-files branch July 14, 2019 11:23
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