Merged
Conversation
Flix6x
requested changes
Dec 21, 2024
Collaborator
Flix6x
left a comment
Flix6x
left a comment
There was a problem hiding this comment.
Thanks! Please also check your own PR's diff, and provide some review instructions. This is a large PR of several thousand lines. My first look tells me there are build files included that shouldn't be included. Please remove those.
…blepower/s2-python into Dev-VladIftime-Kiflin-PPBC
Flix6x
reviewed
Dec 21, 2024
Collaborator
Flix6x
left a comment
Flix6x
left a comment
There was a problem hiding this comment.
The test files that were touched are all about FRBC?
| elif type(field_type).__name__ == "_LiteralGenericAlias": | ||
| value = field_type.__args__[0] | ||
| else: | ||
| breakpoint() |
VladIftime
commented
Jan 4, 2025
VladIftime
commented
Jan 4, 2025
…gnore comments in PPBCPowerSequenceContainerStatu
…Union for optional fields
VladIftime
changed the title
Flix6x
requested changes
Jan 11, 2025
Collaborator
Flix6x
left a comment
Flix6x
left a comment
There was a problem hiding this comment.
Thanks. It looks like we could merge this soon without problems. I only request clearing up some of the TODOs.
src/s2python/s2_control_type.py
Outdated
| self, conn: "S2Connection", msg: S2Message, send_okay: typing.Callable[[], None] | ||
| ) -> None: ... | ||
|
|
||
| # TODO |
Collaborator
There was a problem hiding this comment.
These TODOs should mention an action or a GitHub Issue.
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Signed-off-by: F.N. Claessen <felix@seita.nl>
Flix6x
pushed a commit
that referenced
this pull request
Jan 13, 2025
* PPBC * Generated classes * PPBC sub-modules created * PPBC message types * PPBC fixed issues with wrong class names and the gen_unit tests * Removed the build files * Ignore the randomly generated unit tests * Stop tracking ignored files * Refactor FRBC for better readability and add TODO comments for abstract methods * Remove unused imports from PPBC modules * Added the frbcs tests back * Added the frbcs tests back * Reverted example_frbc_type.py to put it on a different PR * Reverted example_frbc_type.py to put it on a different PR * Reverted src/s2python/generated/gen_s2.py to put it on a different PR * Reverted development_utilities/gen_unit_test_template.py to put it on a different PR * Reverted the frbc unit tests to put them on a different PR * Update pylint configuration to disable W0511 for TODOs and fix type ignore comments in PPBCPowerSequenceContainerStatu * Refactor type annotations in PPBCPowerSequenceContainerStatus to use Union for optional fields * Add type ignore comments for assignment in PPBC model fields * Remove unused import of Duration in ppbc_power_sequence_container_status.py * Update src/s2python/s2_control_type.py * fix: replace generic TODOs with developer instructions Signed-off-by: F.N. Claessen <felix@seita.nl> * chore: check for generic TODOs Signed-off-by: F.N. Claessen <felix@seita.nl> * chore: pylint W0107 Signed-off-by: F.N. Claessen <felix@seita.nl> * chore: pylint W0107 again Signed-off-by: F.N. Claessen <felix@seita.nl> * W2301 from pylint fix --------- Signed-off-by: F.N. Claessen <felix@seita.nl> Co-authored-by: Felix Claessen <30658763+Flix6x@users.noreply.github.com> Co-authored-by: F.N. Claessen <felix@seita.nl> (cherry picked from commit 671f0e9) Signed-off-by: F.N. Claessen <felix@seita.nl>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 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.
No description provided.