Joss-reviews: [REVIEW]: General binary file parser.

[whedon]

Submitting author: @jfjlaros (J.F.J. Laros)
Repository: https://github.com/jfjlaros/bin-parser
Version: 0.0.20
Editor: @danielskatz
Reviewer: @hainesr, @Smattr
Archive: 10.5281/zenodo.1295625

Status

Status badge code:

HTML: <a href="http://joss.theoj.org/papers/345778df6874891e84c712cc0d665549"><img src="http://joss.theoj.org/papers/345778df6874891e84c712cc0d665549/status.svg"></a>
Markdown: [![status](http://joss.theoj.org/papers/345778df6874891e84c712cc0d665549/status.svg)](http://joss.theoj.org/papers/345778df6874891e84c712cc0d665549)

Reviewers and authors:

Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)

Reviewer instructions & questions

@hainesr & @Smattr, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:

1. Make sure you're logged in to your GitHub account
2. Be sure to accept the invite at this URL: https://github.com/openjournals/joss-reviews/invitations

The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @danielskatz know.

Review checklist for @hainesr

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: Does the release version given match the GitHub release (0.0.20)?
• [x] Authorship: Has the submitting author (@jfjlaros) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?
• [x] Functionality: Have the functional claims of the software been confirmed?
• [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [x] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

Review checklist for @Smattr

Conflict of interest

• [x] As the reviewer I confirm that I have read the JOSS conflict of interest policy and that there are no conflicts of interest for me to review this work.

Code of Conduct

• [x] I confirm that I read and will adhere to the JOSS code of conduct.

General checks

• [x] Repository: Is the source code for this software available at the repository url?
• [x] License: Does the repository contain a plain-text LICENSE file with the contents of an OSI approved software license?
• [x] Version: Does the release version given match the GitHub release (0.0.20)?
• [x] Authorship: Has the submitting author (@jfjlaros) made major contributions to the software? Does the full list of paper authors seem appropriate and complete?

Functionality

• [x] Installation: Does installation proceed as outlined in the documentation?
• [x] Functionality: Have the functional claims of the software been confirmed?
• [x] Performance: If there are any performance claims of the software, have they been confirmed? (If there are no claims, please check off this item.)

Documentation

• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] Installation instructions: Is there a clearly-stated list of dependencies? Ideally these should be handled with an automated package management solution.
• [x] Example usage: Do the authors include examples of how to use the software (ideally to solve real-world analysis problems).
• [x] Functionality documentation: Is the core functionality of the software documented to a satisfactory level (e.g., API method documentation)?
• [x] Automated tests: Are there automated tests or manual steps described so that the function of the software can be verified?
• [x] Community guidelines: Are there clear guidelines for third parties wishing to 1) Contribute to the software 2) Report issues or problems with the software 3) Seek support

Software paper

• [x] Authors: Does the paper.md file include a list of authors with their affiliations?
• [x] A statement of need: Do the authors clearly state what problems the software is designed to solve and who the target audience is?
• [x] References: Do all archival references that should have a DOI list one (e.g., papers, datasets, software)?

[whedon]

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks. @hainesr, it looks like you're currently assigned as the reviewer for this paper :tada:.

:star: Important :star:

If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿

To fix this do the following two things:

1. Set yourself as 'Not watching' https://github.com/openjournals/joss-reviews:

1. You may also like to change your default settings for this watching repositories in your GitHub profile here: https://github.com/settings/notifications

For a list of things I can do to help you, just type:

@whedon commands

[whedon]

Attempting PDF compilation. Reticulating splines etc...

[whedon]

--> Check article proof :page_facing_up: <--

[danielskatz]

@hainesr & @Smattr - over to you now to start your reviews, following the instructions above.

Please let me know if you have any questions or run into any problems.

[hainesr]

Hi @danielskatz, for the version check, does there need to be an actual tagged release and corresponding GitHub release (i.e. here: https://github.com/jfjlaros/bin-parser/releases)?

[danielskatz]

@hainesr - I think so. @arfon, can you confirm?

[danielskatz]

👋 @arfon - see question above

[Smattr]

@jfjlaros very nice project! The included presentation also gives a very nice motivation for how this project came about. I've posted a series of issues resulting from my review thus far. Some high level questions for you and perhaps @arfon as well:

1. Would you like us to submit nitpick feedback on coding style/conventions as well? E.g. more Pythonic idioms for doing certain things. If so, should these come as a separate issue or a PR or something else?
2. There seems to be very little error handling. I triggered multiple exceptions with malformed data or structure descriptions. This seems fine to me as this tool is a reversing aid not meant for end users, but perhaps JOSS has guidelines on this? From a personal standpoint, I think reversers generally prefer a bare exception trace as it helps you better debug your spec and incrementally reverse a format.

[jfjlaros]

Dear @Smattr,

Thank you for your kind words and your thorough review. I will try to address the issues you raised, and perhaps implement some of the features you requested. The latter will depend mainly on implementation time, but I think some can be addressed rather quickly.

As for detailed feedback, it is certainly welcome. I try to follow the recommended coding styles of the two programming languages used, but in this particular case I also try to keep the two code bases as similar as possible for maintainability reasons. This may result in some awkward code here and there. In any case, I would appreciate both general comments about my coding style and detailed remarks in any form you see fit.

I expected remarks about error handling and like you I am still on the fence on whether to implement it.

[danielskatz]

@Smattr

1. Nitpicking items (that don't affect your checking boxes in the review, but would be nice to have) should probably be issues or PRs in the repo.
2. Re error handling - if you think this should block acceptance (ie you think the software really needs to have this to be useful), then let us know that - otherwise, again an issue in the repo that informs the developer but doesn't affect this submission would be good

[Smattr]

I think it's fine to leave as-is with regards to the error handling. I'll submit a PR with my nitpicks and we can discuss in more detail on that. Thanks, both.

[arfon]

Hi @danielskatz, for the version check, does there need to be an actual tagged release and corresponding GitHub release (i.e. here: https://github.com/jfjlaros/bin-parser/releases)?

@hainesr - I think so. @arfon, can you confirm?

Yes please.

[hainesr]

Thanks @arfon. I have raised an issue.

[hainesr]

@danielskatz - I think I am happy now, with the proviso that jfjlaros/bin-parser#18 is fixed once the other suggestions have been made.

@jfjlaros - This is a really nice tool. I don't have a use-case for something like this right now, but it would have made my life easier numerous times in the past.

[jfjlaros]

Dear @hainesr,

Thank you very much for your suggestions and fixes.

[hainesr]

@danielskatz, @arfon - There is a question about whether the version pointed to by this submission can change now there have been lots of updates to this project here: jfjlaros/bin-parser#18.

[danielskatz]

yes, it's fine to change the version, in both the paper.md and the repo

[Smattr]

At this point, I'm happy with the submission after the Python 3 compatibility branch is merged (https://github.com/jfjlaros/bin-parser/issues/8) and we have a tagged version (https://github.com/jfjlaros/bin-parser/issues/18). Well done addressing everything so fast, @jfjlaros.

[jfjlaros]

I released v1.0.0.

Thanks @Smattr, @hainesr for your suggestions and contributions. It really improved the quality of the package.

[jfjlaros]

I added credits in c1f2e6e, do we also want these in the release?

[danielskatz]

@jfjlaros , I think that's up to you.

Let me know if you think this is ready to publish, or if you want to make any final changes.

[jfjlaros]

@danielskatz, I added the credits and automatic testing with Travis to v1.0.1. I think that wraps it up.

[danielskatz]

Is 1.0.1 archived somewhere with a DOI? If not, please do this.

And then paste the DOI here.

[jfjlaros]

I archived it at Zenodo: 10.5281/zenodo.1295625.

[danielskatz]

@whedon commands

[whedon]

Here are some things you can ask me to do:

# List all of Whedon's capabilities
@whedon commands

# Assign a GitHub user as the sole reviewer of this submission
@whedon assign @username as reviewer

# Add a GitHub user to the reviewers of this submission
@whedon add @username as reviewer

# Remove a GitHub user from the reviewers of this submission
@whedon remove @username as reviewer

# List of editor GitHub usernames
@whedon list editors

# List of reviewers together with programming language preferences and domain expertise
@whedon list reviewers

# Change editorial assignment
@whedon assign @username as editor

# Set the software archive DOI at the top of the issue e.g.
@whedon set 10.0000/zenodo.00000 as archive

# Open the review issue
@whedon start review

🚧 🚧 🚧 Experimental Whedon features 🚧 🚧 🚧

# Compile the paper
@whedon generate pdf

[danielskatz]

@whedon set 10.5281/zenodo.1295625 as archive

[whedon]

OK. 10.5281/zenodo.1295625 is the archive.

[danielskatz]

👋 @arfon - over to you for final acceptance

@Smattr, @hainesr - Thanks for your reviewing and contributions!

[arfon]

@hainesr, @Smattr - many thanks for your reviews here and to @danielskatz for editing this submission ✨

@jfjlaros - your paper is now accepted into JOSS and your DOI is https://doi.org/10.21105/joss.00766 :zap: :rocket: :boom:

[whedon]

:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:

If you would like to include a link to your paper from your README use the following code snippet:

[![DOI](http://joss.theoj.org/papers/10.21105/joss.00766/status.svg)](https://doi.org/10.21105/joss.00766)

This is how it will look in your documentation:

We need your help!

Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:

• Volunteering to review for us sometime in the future. You can add your name to the reviewer list here: http://joss.theoj.org/reviewer-signup.html
• Making a small donation to support our running costs here: https://www.flipcause.com/secure/cause_pdetails/Mjk3Nzk=

[jfjlaros]

@arfon, @danielskatz: Thank you for editing this submission.

@Smattr, @hainesr: Thank you for your suggestions and contributions. I really appreciate it.

[Smattr]

This was the first JOSS review I've been involved with and I thought the whole thing went very well. Thanks all :)