Refactor BlockFactory interface #1045
Merged
Conversation
This file contains hidden or 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
Change-Id: I7e9aa60f541c00852c548332338ee0fc914ee92f
Codecov Report
@@ Coverage Diff @@
## master #1045 +/- ##
============================================
- Coverage 70.36% 70.35% -0.01%
+ Complexity 5168 5166 -2
============================================
Files 508 509 +1
Lines 23084 23080 -4
Branches 2429 2428 -1
============================================
- Hits 16242 16237 -5
Misses 5536 5536
- Partials 1306 1307 +1
Continue to review full report at Codecov.
|
frankfliu
requested review from
AzizZayed,
aksrajvanshi,
lanking520,
stu1130 and
zachgk
zachgk
reviewed
Jun 23, 2021
| * @throws IOException if IO operation fails during creating block | ||
| */ | ||
| Block newBlock(NDManager manager); | ||
| Block newBlock(Model model, Path modelPath, Map<String, ?> arguments) throws IOException; |
Contributor
There was a problem hiding this comment.
Is this right? I thought the plan was to use BlockFactory to deprecate the Map<String, ?> arguments and replace it with serializing the BlockFactory?
Contributor
Author
There was a problem hiding this comment.
The purpose of BlockFactory is to create the Block then you can load the parameters into the Block.
The problem is the BlockFactory may need extra information:
- Some files in the model directory
- some model specific arguments. If we hard-code those arguments, which will make the BlockFactory implementation not re-useable.
zachgk
approved these changes
Jun 23, 2021
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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-Id: I7e9aa60f541c00852c548332338ee0fc914ee92f
Description
Brief description of what this PR is about