This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.6k
Remove any test-helpers
features
#9727
Labels
I7-refactor
Code needs refactoring.
Comments
bkchr
added a commit
that referenced
this issue
Dec 30, 2021
`test-helpers` feature is a bad idea in general, because once the feature is enabled somewhere in the workspace, it is enabled anywhere. While removing the feature, the tests were also rewritten to get rid off other "only test" related code. Contributes towards: #9727
paritytech-processbot bot
pushed a commit
that referenced
this issue
Jan 3, 2022
* Remove transaction-pool `test-helpers` feature `test-helpers` feature is a bad idea in general, because once the feature is enabled somewhere in the workspace, it is enabled anywhere. While removing the feature, the tests were also rewritten to get rid off other "only test" related code. Contributes towards: #9727 * Apply suggestions from code review Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> * Fix benches Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
grishasobol
pushed a commit
to gear-tech/substrate
that referenced
this issue
Mar 28, 2022
* Remove transaction-pool `test-helpers` feature `test-helpers` feature is a bad idea in general, because once the feature is enabled somewhere in the workspace, it is enabled anywhere. While removing the feature, the tests were also rewritten to get rid off other "only test" related code. Contributes towards: paritytech#9727 * Apply suggestions from code review Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> * Fix benches Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
ark0f
pushed a commit
to gear-tech/substrate
that referenced
this issue
Feb 27, 2023
* Remove transaction-pool `test-helpers` feature `test-helpers` feature is a bad idea in general, because once the feature is enabled somewhere in the workspace, it is enabled anywhere. While removing the feature, the tests were also rewritten to get rid off other "only test" related code. Contributes towards: paritytech#9727 * Apply suggestions from code review Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> * Fix benches Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Remove any feature that re-exports testing stuff when the feature is activated because it is useless. A proper approach is either to move testing stuff into its own crate or to just export it by default. Having stuff "hidden" behind a feature doesn't work in workspaces and is a bad practice anyway. When a feature is enabled in a workspace, it will be automatically enabled everywhere, defeating the whole purpose of such a feature.
The best example is the
sc-service
crate that tries to hide the client, which is actually accessible when enabling the feature. As the feature is enabled all the time, no one knows that the client is hidden behind such a feature.The text was updated successfully, but these errors were encountered: