Update Svc/PolyDb to use configurable FPP enumeration as index
#2587
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
LeStarch
reviewed
Mar 12, 2024
config/PolyDbImplCfg.hpp
Outdated
| enum { | ||
| POLYDB_NUM_DB_ENTRIES = 25 | ||
| }; | ||
| // enum { |
There was a problem hiding this comment.
If the entry count is inferred from the number of entries in the enum, then delete this whole file.
Svc/PolyDb/test/ut/Readme.txt
Outdated
| @@ -1,9 +1,9 @@ | |||
| This test can be run by executing the following: | |||
There was a problem hiding this comment.
Just delete this file? We no longer maintain per-component readmes.
| @@ -0,0 +1,38 @@ | |||
| // ====================================================================== | |||
There was a problem hiding this comment.
You could take this opportunity to switch to the auto-helpers setup which autocodes this file.
Svc/PolyDb/PolyDb.cpp
Outdated
| } | ||
|
|
||
| void PolyDb::init(NATIVE_INT_TYPE instance) { | ||
| PolyDbComponentBase::init(instance); |
There was a problem hiding this comment.
Delete the whole init function. We have been removing them and using the parent init
LeStarch
approved these changes
Mar 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change Description
Update
Svc/PolyDbto use an FPP enum for the database index. PolyDb implementation files were also updated to the new file naming convention.Rationale
Using an FPP enum allows projects to assign names to the index as opposed to using just a bare integer. This will make the code clearer and less error-prone. Moving the index enum to the config directory separates it from the implementation so projects can customize it.
Testing/Review Recommendations
Unit testing was update to the new file organization and helper classes. The nominal unit test code was essentially unchanged except for updating to use the enumeration class.
Future Work
This enables the future sequencing proposals to have conditional logic based on
PolyDbentries. The sequences can have entries by name using the FPP enum to make it clearer.