search

bit-dream / candied

A zero dependency DBC parser written in pure Javascript/Typescript
MIT License
33 stars 10 forks source link

Implemented `encode` functionality #55

Closed EFeru closed 2 months ago

EFeru commented 3 months ago

Hi @bit-dream,

I implemeted the missing encode functionality including tests. Do you mind having a look if all is ok and merge? I would really appreciate a release after merge.

Implements #7.

Let me know your feedback, Emanuel

EFeru commented 2 months ago

Hi @tylerbucher , Any idea who can merge this PR? I am not sure if @bit-dream is still available to support this repository. Thanks for your answer.

tylerbucher commented 2 months ago

@EFeru I left a review but I do not have write permissions to be able to merge your PR.

EFeru commented 2 months ago

@tylerbucher indeed there is room for improvement, especially there are some repetitive tasks that could essentially be done once, like payload2Binary and then binary2Payload. I think a better approach would be to do once payload2Binary then insertBitRange for all signals and then binary2Payload once. However, for flexibility and as a first working solution I think is a good start.

I am not sure when you said you left a review, if you gave specific code review or just the comment that I see above, regarding the performance?

tylerbucher commented 2 months ago

I reviewed you code and I was just saying it looks good enough to merge but I had an out standing comment that should not prevent this PR from merging.

bit-dream commented 2 months ago

@EFeru changeset looks good to me as well. I'll merge in your PR.

@tylerbucher I invited you as a collaborator to the repository, once you are in I can give you write access.

EFeru commented 2 months ago

Sounds great @bit-dream . Thanks for your effort!