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

mempool: Move to internal. #2274

Merged
merged 2 commits into from
Jul 21, 2020
Merged

mempool: Move to internal. #2274

merged 2 commits into from
Jul 21, 2020

Conversation

davecgh
Copy link
Member

@davecgh davecgh commented Jul 19, 2020

This requires #2273.

This makes the mempool an internal package such that it will no longer be an exported module. The only place its functionality is really needed is for the internal implementation of dcrd itself and thus having an unnecessary exported module significantly increases the maintenance burden.

This is part of the overall effort to reduce the total number of modules and eventually get to the point it will be possible to follow semver for the root module.

It is worth noting that there are a few constants, which will be listed below, that were exported from this module that callers might have previously relied upon. However, due to previous discussions about the goal of removing the module, a code search has revealed that there are no known callers that still rely on them.

The aforementioned constants are:

  • MaxStandardTxSize
  • DefaultMinRelayTxFee
  • BaseStandardVerifyFlags

Overview of the major changes:

  • Move the following files from mempool -> internal/mempool:
    • README.md
    • doc.go
    • error.go
    • log.go
    • mempool.go
    • mempool_test.go
    • policy.go
    • policy_test.go
  • Remove mempool/go.mod and mempool/go.sum
  • Make the README.md and doc.go files match the new reality
  • Update all import paths in the repository accordingly
  • Remove the dependency from the root go.mod
  • Run go mod tidy on all modules
  • Update project docs for removal of the module.

@davecgh davecgh added this to the 1.6.0 milestone Jul 19, 2020
Copy link
Member

@rstaudt2 rstaudt2 left a comment

Choose a reason for hiding this comment

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

Looks good to me. Did notice one minor typo in the first commit/PR description:
s/The/This is part of the overall effort

Copy link
Member

@dnldd dnldd left a comment

Choose a reason for hiding this comment

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

Besides needing a rebase this looks good.

@davecgh
Copy link
Member Author

davecgh commented Jul 19, 2020

Rebased and fixed the commit message typo pointed out by @rstaudt2.

@JoeGruffins
Copy link
Member

JoeGruffins commented Jul 20, 2020

At the end of the file at dcrd/internal/mempool/README.md There is a section Installation and Updating that should be removed or changed to say somethig like This package is internal and therefore is neither directly installed nor needs to be manually updated..

@JoeGruffins
Copy link
Member

JoeGruffins commented Jul 20, 2020

Not completely sure, but in the commit message This is part of the overall effort to reduce the total number of modules and eventually get to the point it will be possible to follow semver for the root module. I think needs to be ... get to the point where/that/at which it will be ...

This makes the mempool an internal package such that it will no longer
be an exported module.  The only place its functionality is really
needed is for the internal implementation of dcrd itself and thus having
an unnecessary exported module significantly increases the maintenance
burden.

This is part of the overall effort to reduce the total number of modules
and eventually get to the point it will be possible to follow semver for
the root module.

It is worth noting that there are a few constants, which will be listed
below, that were exported from this module that callers might have
previously relied upon.  However, due to previous discussions about the
goal of removing the module, a code search has revealed that there are
no known callers that still rely on them.

The aforementioned constants are:

- MaxStandardTxSize
- DefaultMinRelayTxFee
- BaseStandardVerifyFlags

Overview of the major changes:

- Move the following files from mempool -> internal/mempool:
  - README.md
  - doc.go
  - error.go
  - log.go
  - mempool.go
  - mempool_test.go
  - policy.go
  - policy_test.go
- Remove mempool/go.mod and mempool/go.sum
- Make the README.md and doc.go files match the new reality
- Update all import paths in the repository accordingly
- Remove the dependency from the root go.mod
- Run go mod tidy on all modules
@davecgh
Copy link
Member Author

davecgh commented Jul 20, 2020

@JoeGruffins Regarding the README.md, good catch. I did that for the mining one, but forgot for mempool. Updated.

Regarding the other one, it seems fine to me with or without the word where.

@davecgh davecgh merged commit a524703 into decred:master Jul 21, 2020
@davecgh davecgh deleted the mempool_internal branch July 21, 2020 17:26
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

5 participants