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

Expose BlockLocator #1762

Closed
dahlia opened this issue Jan 26, 2022 · 6 comments
Closed

Expose BlockLocator #1762

dahlia opened this issue Jan 26, 2022 · 6 comments
Labels
good first issue Good for newcomers hacktoberfest Newcomer-welcoming issues for Hacktoberfest

Comments

@dahlia
Copy link
Contributor

dahlia commented Jan 26, 2022

It will be great if we can remove this and make Libplanet internals used in Libplanet.Net public.

Originally posted by @limebell in #1760 (comment)

Currently Libplanet allows Libplanet.Net to see its internals in order to keep BlockLocator internal. However, it would be better if it is exposed as a public class instead.

@stale
Copy link

stale bot commented Apr 16, 2022

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label Apr 16, 2022
@riemannulus
Copy link
Member

This issue is extreamly easy.

The goal of this issue is not to focus on understanding the structure of libplanet, but rather to follow the process of contributing to libplanet.
We are looking forward to familiarizing you with the changelog or lint.

@stale stale bot removed the stale The issue is stale label Jun 3, 2022
@tkiapril
Copy link
Contributor

Is the objective of this issue simply making the class BlockLocator public, or removing the lines:

[assembly: InternalsVisibleTo("Libplanet.Net")]
[assembly: InternalsVisibleTo("Libplanet.Net.Tests")]

?

I'm asking because if it's the former it would be really simple, but in case of the latter, the members of the class BlockChain Swap(other, render, stateCompleters), TipChanged, Store, Append(block, evaluateActions, renderBlocks), FindNextHashes(locator, stop, count), Fork(point, inheritRenderers, GetBlockLocator(threshold), IterateBlockHashes(offset, limit) must also be made public, and the code must be swapped around due to the fact that public members should precede the internal members. I wasn't so sure if this is what you would have wanted.

@greymistcube
Copy link
Contributor

The goal of this issue is not to focus on understanding the structure of libplanet, but rather to follow the process of contributing to libplanet.

Baby steps. 🙂

As the original #1760 (comment) suggest, the goal is eventually to decouple Libpanet and Libplanet.Net, but doing so in one go would be rather hard as you've mentioned.

For now, I think just making partially mentioned entities public and moving around some chunks to pass code analysis would be fine. 😗

@stale
Copy link

stale bot commented Jun 18, 2023

This issue has been automatically marked as stale because it has not had recent activity. Thank you for your contributions.

@stale stale bot added the stale The issue is stale label Jun 18, 2023
@Atralupus Atralupus added the hacktoberfest Newcomer-welcoming issues for Hacktoberfest label Oct 10, 2024
@stale stale bot removed the stale The issue is stale label Oct 10, 2024
@OnedgeLee
Copy link
Contributor

BlockLocator is public already.
If we want to remove internal visibility dependency, then that have to be handled by other issue that has been named properly.
Closing, since this issue has been already resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers hacktoberfest Newcomer-welcoming issues for Hacktoberfest
Projects
Development

No branches or pull requests

7 participants