search

openjournals / joss-reviews

Reviews for the Journal of Open Source Software
Creative Commons Zero v1.0 Universal
725 stars 38 forks source link

[REVIEW]: udpPacketManager: An International LOFAR Station Data (Pre-)Processor #5517

Closed editorialbot closed 7 months ago

editorialbot commented 1 year ago

Submitting author: !--author-handle-->@David-McKenna<!--end-author-handle-- (David McKenna) Repository: https://github.com/David-McKenna/udpPacketManager/ Branch with paper.md (empty if default branch): Version: 0.9.2 Editor: !--editor-->@dfm<!--end-editor-- Reviewers: @pritchardn, @plaplant Archive: 10.5281/zenodo.11019139

Status

status

Status badge code:

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

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) by leaving comments 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

@shmookey & @pritchardn, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review. First of all you need to run this command in a separate comment to create the checklist:

@editorialbot generate my checklist

The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @plaplant know.

✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨

Checklists

📝 Checklist for @pritchardn

📝 Checklist for @plaplant

editorialbot commented 1 year ago

Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks.

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

@editorialbot commands

For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:

@editorialbot generate pdf
editorialbot commented 1 year ago 
Software report:

github.com/AlDanial/cloc v 1.88  T=0.08 s (830.3 files/s, 296771.0 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
C                               19           1933           2726           7530
C++                              7            874            443           3827
C/C++ Header                    16            475            502           1858
Markdown                         7            242              0            892
CMake                            3             64             43            265
TeX                              1             18              0            231
YAML                             4             10              0            145
Python                           1             38             46            137
Bourne Shell                     3             37             15             95
Dockerfile                       1             10             20             26
JSON                             1              1              0             16
-------------------------------------------------------------------------------
SUM:                            63           3702           3795          15022
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 1 year ago

Wordcount for paper.md is 864

editorialbot commented 1 year ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S1743921312024623 is OK
- 10.1071/AS10021 is OK
- 10.1109/JPROC.2004.840301 is OK
- 10.1088/1538-3873/ab3e82 is OK
- 10.5281/ZENODO.7871665 is OK
- 10.1093/rasti/rzac005 is OK
- 10.48550/ARXIV.2302.12661 is OK
- 10.1051/0004-6361/202140415 is OK
- 10.1051/0004-6361/201220873 is OK
- 10.17487/RFC8878 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 1 year ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

pritchardn commented 1 year ago

Review checklist for @pritchardn

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

pritchardn commented 1 year ago

Hello all

I plan on finishing my review by the end of June and will add comments as I see them.

plaplant commented 1 year ago

@shmookey when you get a chance, please reply to this comment with:

@editorialbot generate my checklist

This will create your reviewer checklist that you can then begin going through. Please let me know if you have any questions!

pritchardn commented 1 year ago

A few minor comments on the Software Paper (mainly typos) which is otherwise excellent:

pritchardn commented 1 year ago

I've tried to install the library today, but a problem installing psrdada is preventing me. This looks like it's on their end, however, so I'll try again in a few days

David-McKenna commented 1 year ago

Hey @pritchardn,

Thanks! I've made changes to the paper and the general documentation comments you made in the docsPass branch (PR David-McKenna/udpPacketManager#13 ).

As for the PSRDADA issue, are sourceforge's certs out of date again? That's been a chronic issue over the past few years. I'll see if there's another way for me to download their source in my CMake script.

Cheers, David

plaplant commented 1 year ago

@shmookey when you get a chance, please begin your review of this package. You can get your reviewer checklist by responding to this thread with:

@editorialbot generate my checklist

If you feel that you are no longer able to review, please let me know, and I can work on finding another reviewer. Thanks!

arfon commented 1 year ago

@plaplant – I think it might be time to find a different reviewer here?

plaplant commented 1 year ago

@arfon yes, I agree. I've actually been working behind the scenes to reach out to other potential reviewers, but have been coming up empty. I will reach out to additional folks and see if I can get someone to volunteer. Thanks for keeping up with this!

plaplant commented 1 year ago

Given that this submission has languished a bit, I'm going to go ahead and step in as a reviewer here to make sure the submission keeps moving forward. To keep things conflict-of-interest-free, @dfm has graciously agreed to take over as editor. I will review this as soon as I can, and hopefully we can get this fully reviewed soon.

@David-McKenna thanks so much for your patience thus far! Please let me (or @dfm) know if you have any questions.

plaplant commented 1 year ago

@editorialbot assign @plaplant as reviewer

editorialbot commented 1 year ago

I'm sorry human, I don't understand that. You can see what commands I support by typing:

@editorialbot commands

plaplant commented 1 year ago

@editorialbot add @plaplant as reviewer

editorialbot commented 1 year ago

@plaplant added to the reviewers list!

plaplant commented 1 year ago

Review checklist for @plaplant

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

dfm commented 12 months ago

@editorialbot assign me as editor

editorialbot commented 12 months ago

Assigned! @dfm is now the editor

dfm commented 10 months ago

@editorialbot remove @shmookey from reviewers

@plaplant, @pritchardn, @David-McKenna — Hi all! I wanted to check in here to see how everything is going with this review. It looks like there are a few remaining checklist items, but we're getting pretty close. Is there anything I can do to help us over the finish line? Any specific sticking points?

editorialbot commented 10 months ago

@shmookey removed from the reviewers list!

plaplant commented 10 months ago

@dfm thanks for checking in!

@David-McKenna I recently built the package for my review, and I started by trying to build it inside of an Ubuntu image on my ARM-based Mac using multipass. The compilation failed, but I think it was due to not being an x86-based machine. Building on an x86 system worked as intended, but I did not have enough privileges on that machine to run the tests. That's the reason I haven't checked the "functionality verification" box in my review. (Also, I'm not sure if it was a problem in my setup, or something explicitly about needing an x86 system. If the latter, maybe that's worth mentioning briefly in the docs?)

I saw as part of the GitHub actions the tests are being run there, so I'd be happy to count that as verifying the functionality. If something more independent is required, I'll have to poke around and see if I can get access to a suitable machine. (This is perhaps indirectly a question for @dfm.)

dfm commented 10 months ago

I saw as part of the GitHub actions the tests are being run there, so I'd be happy to count that as verifying the functionality. If something more independent is required, I'll have to poke around and see if I can get access to a suitable machine.

Thanks for bringing this up, @plaplant. From my perspective, I think this is okay as long as the architecture constraints are clearly documented!

pritchardn commented 10 months ago

Hi all, I've tried again (successfully) to install locally and run the included tests. I'm happy to pass this one :+1:

David-McKenna commented 10 months ago

Hi all,

Apologies for my silence over the past few days.

I'm currently in the middle of moving country and starting a new role, so I won't have a chance to address the above requests for another few days, though I hope to get to them by the end of the week.

Cheers, David

On Thu, 1 Feb 2024 at 04:10, Nicholas Pritchard @.***> wrote:

Hi all, I've tried again (successfully) to install locally and run the included tests. I'm happy to pass this one 👍

— Reply to this email directly, view it on GitHub https://github.com/openjournals/joss-reviews/issues/5517#issuecomment-1920417791, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKLPGON6PM7QSS46YBKVKLLYRMBQ3AVCNFSM6AAAAAAYYQE7N2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSMRQGQYTONZZGE . You are receiving this because you were mentioned.Message ID: @.***>

David-McKenna commented 9 months ago

Apologies for the delay, I was expecting to get a work laptop with an ARM CPU in the past few weeks so I could test the described issues, but there's been a hold-up with delivery. For the time being, I've added an additional note that currently only x86-64 architectures are supported (58a2375).

Cheers, David

dfm commented 8 months ago

@plaplant — Can you take another look at this ASAP so that we can get it over the line? Thanks!

plaplant commented 8 months ago

@dfm apologies for the delay. I've updated my checklist and it looks good to go for me!

@David-McKenna congrats on the very nice work!

dfm commented 8 months ago

@editorialbot generate pdf

dfm commented 8 months ago

@editorialbot check references

editorialbot commented 8 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

editorialbot commented 8 months ago 
Reference check summary (note 'MISSING' DOIs are suggestions that need verification):

OK DOIs

- 10.1017/S1743921312024623 is OK
- 10.1071/AS10021 is OK
- 10.1109/JPROC.2004.840301 is OK
- 10.1088/1538-3873/ab3e82 is OK
- 10.5281/ZENODO.7871665 is OK
- 10.1093/rasti/rzac005 is OK
- 10.48550/ARXIV.2302.12661 is OK
- 10.1051/0004-6361/202140415 is OK
- 10.1051/0004-6361/201220873 is OK
- 10.17487/RFC8878 is OK

MISSING DOIs

- No DOI given, and none found for title: LOFAR Und MPIfR Pulsare
- No DOI given, and none found for title: Station Data Cookbook
- No DOI given, and none found for title: dreamBeam
- No DOI given, and none found for title: LOFAR Data Format ICD Beam-Formed Data
- No DOI given, and none found for title: Simultaneous Dual-Site SETI with LOFAR Internation...
- No DOI given, and none found for title: Making Observations with Mode-357
- No DOI given, and none found for title: Pelican/Pelican-Lofar
- No DOI given, and none found for title: PSRDADA: Distributed Acquisition and Data Analysis...
- No DOI given, and none found for title: SIGPROC: Pulsar Signal Processing Programs

INVALID DOIs

- None
dfm commented 8 months ago

@pritchardn, @plaplant — Thanks for your thorough and constructive reviews!!

@David-McKenna — I've opened a small PR with some minor edits to the manuscript, please take a look and merge or let me know what you think.

Once you've done that:

  1. Take one last read through the manuscript to make sure that you're happy with it (it's harder to make changes later!), especially the author names and affiliations. I've taken a pass and it looks good to me!
  2. Increment the version number of the software and report that version number back here.
  3. Create an archived release of that version of the software (using Zenodo or something similar). Please make sure that the metadata (title and author list) exactly match the paper. Then report the DOI of the release back to this thread.
dfm commented 7 months ago

@David-McKenna — Checking in here. We're so close to publication, could up update me on the status of these steps that I asked for above?

David-McKenna commented 7 months ago

Hey @dfm, Apologies, I apparently forgot to swap my GitHub account to my new work email so I never saw the prior comments. I'll get this handled by the end of today.

David-McKenna commented 7 months ago

Tagged and released 0.9.2 with your proposed changes, the Zenodo DOI is 10.5281/zenodo.11019139.

Cheers, David

dfm commented 7 months ago

@David-McKenna — Thanks! Can you update the metadata for the Zenodo deposit (there should be an "edit" button on the top right corner of that page) so that the title and author list match the paper?

David-McKenna commented 7 months ago

Whoops, should be fixed up now.

dfm commented 7 months ago

@editorialbot set 10.5281/zenodo.11019139 as archive

editorialbot commented 7 months ago

Done! archive is now 10.5281/zenodo.11019139

dfm commented 7 months ago

@editorialbot set 0.9.2 as version

editorialbot commented 7 months ago

Done! version is now 0.9.2

dfm commented 7 months ago

@editorialbot generate pdf

editorialbot commented 7 months ago

:point_right::page_facing_up: Download article proof :page_facing_up: View article proof on GitHub :page_facing_up: :point_left:

dfm commented 7 months ago

@editorialbot recommend-accept

editorialbot commented 7 months ago 
Attempting dry run of processing paper acceptance...
editorialbot commented 7 months ago

:wave: @openjournals/aass-eics, this paper is ready to be accepted and published.

Check final proof :point_right::page_facing_up: Download article

If the paper PDF and the deposit XML files look good in https://github.com/openjournals/joss-papers/pull/5259, then you can now move forward with accepting the submission by compiling again with the command @editorialbot accept