Skip to content

Feature/necessary typing #7

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Apr 11, 2022
Merged

Feature/necessary typing #7

merged 2 commits into from
Apr 11, 2022

Conversation

@Hadatko
Copy link
Member

@Hadatko Hadatko commented Apr 8, 2022

No description provided.

@Hadatko Hadatko requested a review from qcabrol April 8, 2022 03:51
@Hadatko Hadatko self-assigned this Apr 8, 2022
@Hadatko Hadatko changed the base branch from master to devel April 8, 2022 03:51
@Hadatko Hadatko force-pushed the feature/necessaryTyping branch from a8fa2d1 to 0cf16c2 Compare April 8, 2022 03:52
@Hadatko
Added some necessary type definitions and improved parsing globals.
e0e95f4 
Signed-off-by: Cervenka Dusan <cervenka@acrios.com>
@Hadatko Hadatko force-pushed the feature/necessaryTyping branch from 0cf16c2 to e0e95f4 Compare April 8, 2022 04:54
@Hadatko
C language is case sensitive.
c96e7ba 
Signed-off-by: Cervenka Dusan <cervenka@acrios.com>
@qcabrol
Copy link
Collaborator

qcabrol commented Apr 8, 2022 •
edited
Loading 

 ✘ quentin@quentin-M17xR4  ~/aeler/dissect.cstruct/tests   devel  pytest test_basic.py 
=============================================================================================== test session starts ===============================================================================================
platform linux -- Python 3.9.5, pytest-7.1.1, pluggy-1.0.0
rootdir: /home/quentin/aeler/dissect.cstruct
collected 34 items                                                                                                                                                                                                

test_basic.py ..................................                                                                                                                                                            [100%]

=============================================================================================== 34 passed in 0.60s ================================================================================================

I suggest you attach a pipeline if possible to the repo to run it on PRs

qcabrol
qcabrol requested changes Apr 8, 2022
View reviewed changes
Copy link
Collaborator

@qcabrol qcabrol left a comment •
edited
Loading

Choose a reason for hiding this comment

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

image

Broke the documentation generation. Issue might be on our MsgParser.py side but that somehow breaks retro-compatibilty. One side needs fixing.

Actually this was broken by the previous PR, not this one.

dissect/cstruct/__init__.py
WcharType,
UnionType,
VoidType,
typeAll,
Copy link
Collaborator

@qcabrol qcabrol Apr 8, 2022

Choose a reason for hiding this comment

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

typeAll ?

Copy link
Member Author

@Hadatko Hadatko Apr 9, 2022

Choose a reason for hiding this comment

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

typeAll => i created few predefined types to simpliefied typing. This one i exposed.

Copy link
Member Author

@Hadatko Hadatko Apr 9, 2022

Choose a reason for hiding this comment

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

image

@qcabrol
Copy link
Collaborator

qcabrol commented Apr 8, 2022

Regression detected on commit 28f10e0

image

@Hadatko Hadatko requested a review from qcabrol April 11, 2022 13:24
@qcabrol
Copy link
Collaborator

qcabrol commented Apr 11, 2022

Solved with changes on the use case code.

@Hadatko Hadatko merged commit 52fab6b into devel Apr 11, 2022
@Hadatko Hadatko deleted the feature/necessaryTyping branch April 11, 2022 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@qcabrol qcabrol qcabrol approved these changes

Assignees

@Hadatko Hadatko

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Hadatko @qcabrol