openjournals / joss-reviews

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

[REVIEW]: TrackerControl: Transparency and Choice around App Tracking #4270

Closed editorialbot closed 2 years ago

editorialbot commented 2 years ago

Submitting author: !--author-handle-->@kasnder<!--end-author-handle-- (Konrad Kollnig) Repository: https://github.com/TrackerControl/tracker-control-android Branch with paper.md (empty if default branch): master Version: 2022061501 Editor: !--editor-->@sbenthall<!--end-editor-- Reviewers: @gradvohl, @gcdeshpande Archive: 10.5281/zenodo.6645297

Status

status

Status badge code:

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

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

@gradvohl & @gcdeshpande, 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 @sbenthall 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 @gcdeshpande

📝 Checklist for @gradvohl

editorialbot commented 2 years 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 2 years ago

Checking the BibTeX entries failed with the following error:

No paper file path
editorialbot commented 2 years ago
Software report:

github.com/AlDanial/cloc v 1.88  T=0.43 s (450.7 files/s, 87364.2 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
XML                            111            531            211          13305
Java                            51           2229           1486          12258
C                               11            787            480           3893
SVG                              4              0              2           1056
C/C++ Header                     2            145             34            427
Markdown                         3             69              0            188
Gradle                           3             26             16            127
Bourne Shell                     1             27            108             99
DOS Batch                        1             21              2             66
HTML                             1              7              3             43
ProGuard                         1              9             26             28
YAML                             2              4              0             25
Sass                             1              7              0             24
CMake                            1              5              0             19
JSON                             2              0              0              2
-------------------------------------------------------------------------------
SUM:                           195           3867           2368          31560
-------------------------------------------------------------------------------

gitinspector failed to run statistical information for the repository
editorialbot commented 2 years ago

Failed to discover a Statement of need section in paper

editorialbot commented 2 years ago

:warning: An error happened when generating the pdf. Paper file not found.

sbenthall commented 2 years ago

👋🏼 @kasnder @gradvohl, @gcdeshpande this is the review thread for the paper. All of our communications will happen here from now on.

Both reviewers have checklists at the top of this thread with the JOSS requirements. As you go over the submission, please check any items that you feel have been satisfied. There are also links to the JOSS reviewer guidelines.

The JOSS review is different from most other journals. Our goal is to work with the authors to help them meet our criteria instead of merely passing judgment on the submission. As such, the reviewers are encouraged to submit issues and pull requests on the software repository. When doing so, please mention openjournals/joss-reviews#4270 so that a link is created to this thread (and I can keep an eye on what is happening). Please also feel free to comment and ask questions on this thread. In my experience, it is better to post comments/questions/suggestions as you come across them instead of waiting until you've reviewed the entire package.

We aim for reviews to be completed within about 2-4 weeks. Please let me know if any of you require some more time. We can also use EditorialBot (our bot) to set automatic reminders if you know you'll be away for a known period of time.

Please feel free to ping me (@sbenthall) if you have any questions/concerns.

sbenthall commented 2 years ago

@kasnder The editorial bot seems to be having trouble finding your paper in the repository.

Where is your paper located?

kasnder commented 2 years ago

@editorialbot generate pdf from branch joss

editorialbot commented 2 years ago

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

@editorialbot commands

kasnder commented 2 years ago

Hi @sbenthall

Thank you for getting the processed started. The paper is in the branch joss.

gcdeshpande commented 2 years ago

Review checklist for @gcdeshpande

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

sbenthall commented 2 years ago

@editorialbot set joss as branch

editorialbot commented 2 years ago

Done! branch is now joss

sbenthall commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

gradvohl commented 2 years ago

Review checklist for @gradvohl

Conflict of interest

Code of Conduct

General checks

Functionality

Documentation

Software paper

gradvohl commented 2 years ago

Reviewer report for JOSS paper #4270

Paper title: TrackerControl: Transparency and Choice around App Tracking

Review criteria

Note: whenever an item is marked with :warning: or :no_entry:, you can get more information about that evaluation in the Reviewer Comments section).

Review items

Test environment

The app was downloaded and tested on an Android smartphone.

Reviewer comments

The following comments refer to items I considered the author could improve to meet JOSS submission requirements.

About paper.md file

Although the paper.md file is in the joss branch, it will be better if it is in the master branch, especially when the authors release the app's final version.

Mentions any ongoing research projects

I did not find any mention about ongoing research projects. It is not a requirement for JOSS, but if the authors are aware of any project that uses the TrackControl app, I recommend citing that project or, perhaps, writing about its potential in other research projects.

A list of key references

There is a list of key references on the paper.md file. However, there are no references in the repository. I do not think that list is mandatory in the repository, but it is worth checking.

Example usage

I did not find any tutorial or examples for using the software in any file on the repository. Therefore, I suggest that the author include a docs directory with a tutorial or screenshots describing the software's typical usage (step-by-step).

Scholarly effort

The software had a substantial effort to build it. However, I consider that the software is much more commercial than academic software. I highlight that my opinion is not a demerit of the software.

API Docs

I did not find any documentation about the code regarding the API documentation, neither in the source code nor in a docs directory (the repository does not have a docs directory). Therefore, I strongly recommend the documentation of the source code using a standard. My suggestion is to use the Javadocs or Doxygen software. Doxygen can generate the docs (in HTML or PDF) from the documented source files.

Community guidelines

There are indications of communities in different media (Telegram, Matrix etc). However, there are no community guidelines. My suggestion is to create a Contributing section (or a CONTRIBUTING.md file) with more specific instructions about how to contribute.

Tests on source code

We ran the application tests on an Android smartphone. It works nicely. However, we did not find any automatic tests on the source code or instructions about how to do the tests. It is not mandatory to specify those instructions, but it would be nice if the authors provided instructions on how to compile and test the source code.

Other Suggestions

Please, consider including the CITATION.cff file with information about how to cite the TrackerControl software. In addition, information about software citation in GitHub is available on GitHub Docs.

Conclusion

Analyzing all the aforementioned aspects, I rank the paper as major revisions needed. Although most of those revisions are very easy to implement, I consider the lack of documentation and the absence of the paper.md file in the master branch prevents acceptance of the paper at this time.

kasnder commented 2 years ago

Hello @gradvohl,

Thank you for the swift review of our project. These are all great suggestions. I'll do my best to implement them!

editorialbot commented 2 years ago

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

@editorialbot commands

kasnder commented 2 years ago

@editorialbot set master as branch

editorialbot commented 2 years ago

Done! branch is now master

kasnder commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

kasnder commented 2 years ago

A little update: I've now addressed most suggestions. The main missing piece is adding javadocs comments.

Additionally, I might add some screenshots for the now added example use, but then, this might not actually be necessary.

kasnder commented 2 years ago

@gradvohl, thanks again for your swift review of our project and for taking out some of your time to do this. We've now tried to address your key points.

About paper.md file

Although the paper.md file is in the joss branch, it will be better if it is in the master branch, especially when the authors release the app's final version.

Good point! I've moved this to the master branch now.

Mentions any ongoing research projects

I did not find any mention about ongoing research projects. It is not a requirement for JOSS, but if the authors are aware of any project that uses the TrackControl app, I recommend citing that project or, perhaps, writing about its potential in other research projects.

Great suggestion! I've added a list of past and ongoing related research projects at the end of the paper.md file now.

A list of key references

There is a list of key references on the paper.md file. However, there are no references in the repository. I do not think that list is mandatory in the repository, but it is worth checking.

We have a paper that used TrackerControl as its methodology, and developed the underlying tracker database in a range of other publications.

I've added a list of key references to the repository now: https://github.com/TrackerControl/tracker-control-android#citation

Example usage

I did not find any tutorial or examples for using the software in any file on the repository. Therefore, I suggest that the author include a docs directory with a tutorial or screenshots describing the software's typical usage (step-by-step).

That's a great point, thanks! I've added detailed information on example use now: https://github.com/TrackerControl/tracker-control-android#example-use

Scholarly effort

The software had a substantial effort to build it. However, I consider that the software is much more commercial than academic software. I highlight that my opinion is not a demerit of the software.

Thanks for raising this. I think I see where this is coming from, and understand that our submission might be a little bit unusual. I think there are two aspects to the app: 1) the use by non-researchers, which is what drives the community and provides valuable feedback for app development, and 2) the use by researchers for legitimate research studies. On the latter, I've added a section to the paper.md file for clarification now.

API Docs

I did not find any documentation about the code regarding the API documentation, neither in the source code nor in a docs directory (the repository does not have a docs directory). Therefore, I strongly recommend the documentation of the source code using a standard. My suggestion is to use the Javadocs or Doxygen software. Doxygen can generate the docs (in HTML or PDF) from the documented source files.

Good point! I've added javadocs to the source code of the core of TrackerControl now (but not to the code that's been forked from NetGuard, since this is an external project and somewhat unrelated). I've used Doxygen to generate HTML in the docs/ folder.

Community guidelines

There are indications of communities in different media (Telegram, Matrix etc). However, there are no community guidelines. My suggestion is to create a Contributing section (or a CONTRIBUTING.md file) with more specific instructions about how to contribute.

Another great point! I've now added a Contributing section: https://github.com/TrackerControl/tracker-control-android#contributing

Tests on source code

We ran the application tests on an Android smartphone. It works nicely. However, we did not find any automatic tests on the source code or instructions about how to do the tests. It is not mandatory to specify those instructions, but it would be nice if the authors provided instructions on how to compile and test the source code.

This is a good point! We're not doing tests because this is quite tricky in networked environments. I have, however, added build instructions now: https://github.com/TrackerControl/tracker-control-android#build-instructions

Other Suggestions

Please, consider including the CITATION.cff file with information about how to cite the TrackerControl software. In addition, information about software citation in GitHub is available on GitHub Docs.

Thanks, I wasn't even aware of this.. I've added such a file now: https://github.com/TrackerControl/tracker-control-android/blob/master/CITATION.cff

Conclusion

Analyzing all the aforementioned aspects, I rank the paper as major revisions needed. Although most of those revisions are very easy to implement, I consider the lack of documentation and the absence of the paper.md file in the master branch prevents acceptance of the paper at this time.

Please let us know if our changes are adequate, or if you think we should need to improve more.

sbenthall commented 2 years ago

@gcdeshpande and @gradvohl, would you be able to revisit this submission and confirm whether the revisions are adequate? Thank you.

gradvohl commented 2 years ago

I apologize for the delay in revisiting the submission. I am overwhelmed by the classes I am teaching, papers, and projects that I am writing (most of them with fast approaching deadlines). Regarding the TrackerControl app, the authors addressed most of the issues I raised. However, there are some adjustments to those issues. I will point them as follows, using :heavy_check_mark: when it is ok; and :triangular_flag_on_post: when it needs adjustments.

About paper.md file :heavy_check_mark:

Mentions any ongoing research projects: :triangular_flag_on_post:

I am not sure if the authors addressed this issue properly. I did not find any mentions of ongoing projects at the end of the paper.md file. However, the paper mentions some works that served as a base for the TrackerControl app. Nevertheless, it is important to mention other projects that use the app or write about its potential in other research projects.

A list of key references: :triangular_flag_on_post:

I did not find any key references list in the repository. The references are in the paper.md file, but not in the repository README.md file.

BTW, the README.md file has a Citation section. However, that section only shows how users should cite the TrackerControl app.

Example Usage: :heavy_check_mark: (with observations)

It is ok. There is an example there. However, as a user, I would prefer to run the app other than reading all that text about how to use it. Therefore, I suggest that, in future releases, the authors add some screenshots showing typical software usage.

Scholarly effort: :heavy_check_mark:

API Docs: :heavy_check_mark: (with observations)

The authors generated the docs as HTML, but it does not look nice in the repository. If I am not wrong, Doxygen can generate a pdf file, which is more suitable for use, before cloning the repository. Therefore, please consider generating a pdf file in addition.

Community guidelines: :heavy_check_mark:

Tests on source code: :heavy_check_mark:

Conclusion

Considering the revisions made by the authors, I believe the paper is closer to acceptance. Adjusting or justifying the points marked before with :triangular_flag_on_post: will lead to full acceptance, in my opinion.

kasnder commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

kasnder commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

kasnder commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

kasnder commented 2 years ago

Thank you for having another look at our work @gradvohl!

About paper.md file ✔️

Mentions any ongoing research projects: 🚩

I am not sure if the authors addressed this issue properly. I did not find any mentions of ongoing projects at the end of the paper.md file. However, the paper mentions some works that served as a base for the TrackerControl app. Nevertheless, it is important to mention other projects that use the app or write about its potential in other research projects.

I added more details on this now, and a separate section in the paper.md.

A list of key references: 🚩

I did not find any key references list in the repository. The references are in the paper.md file, but not in the repository README.md file.

BTW, the README.md file has a Citation section. However, that section only shows how users should cite the TrackerControl app.

I think I had misunderstood this. I've now added a list of key references to the README.md at https://github.com/TrackerControl/tracker-control-android#references.

API Docs: ✔️ (with observations)

The authors generated the docs as HTML, but it does not look nice in the repository. If I am not wrong, Doxygen can generate a pdf file, which is more suitable for use, before cloning the repository. Therefore, please consider generating a pdf file in addition.

I had some problems generating the pdf, but I've fixed this now and added the result at https://github.com/TrackerControl/tracker-control-android/blob/master/docs/pdf/refman.pdf.

Conclusion

Considering the revisions made by the authors, I believe the paper is closer to acceptance. Adjusting or justifying the points marked before with 🚩 will lead to full acceptance, in my opinion.

Do let us know if we've sufficiently addressed your suggestions, or if there's more work to do.

gradvohl commented 2 years ago

Good evening @kasnder , For me, everything is ok now. Congratulations.

kasnder commented 2 years ago

Good evening @kasnder , For me, everything is ok now. Congratulations.

Great, thank you for your time and support to get to this stage!

sbenthall commented 2 years ago

@gcdeshpande This submission is now waiting on your feedback. Has the submission been corrected to your satisfaction?

sbenthall commented 2 years ago

Hello @gcdeshpande ,

This submission has gone through a round of reviewing by @gradvohl and revisions by the author. But it is still waiting on a second review. Looking at your checklist, is it correct to say that you haven't reviewed the software yet? Would it be possible for you to review the submission in its current status and see if it meets the requirements?

Thank you, @sbenthall

gcdeshpande commented 2 years ago

Hello @sbenthall and @kasnder, Since the author has implemented the suggestions and I don’t find any issues the submission can be accepted.

sbenthall commented 2 years ago

@gcdeshpande Thank you!

The reviewers recommend acceptance and we can now proceed with finalizing the submission!

sbenthall commented 2 years ago

@editorialbot generate pdf

sbenthall commented 2 years ago

@editorialbot check references

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

OK DOIs

- 10.1145/2462456.2464460 is OK
- 10.1145/2787394.2787396 is OK
- 10.1145/2808117.2808120 is OK
- 10.1515/popets-2018-0035 is OK
- 10.1515/popets-2018-0021 is OK
- 10.1145/3173574.3173967 is OK
- 10.1145/3025453.3025556 is OK
- 10.1145/2906388.2906392 is OK
- 10.1515/popets-2018-0035 is OK
- 10.2478/popets-2020-0017 is OK
- 10.1145/3176246 is OK
- 10.1145/3278532.3278558 is OK
- 10.1145/2591971.2592003 is OK
- 10.1145/3201064.3201089 is OK
- 10.14722/ndss.2018.23009 is OK
- 10.2478/popets-2022-0033 is OK
- 10.14763/2021.4.1611 is OK
- 10.1007/978-3-030-78120-0_15 is OK

MISSING DOIs

- None

INVALID DOIs

- None
editorialbot commented 2 years ago

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

sbenthall commented 2 years ago

@kasnder Could you please:

kasnder commented 2 years ago

@editorialbot generate pdf

editorialbot commented 2 years ago

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

kasnder commented 2 years ago

@sbenthall, here is the requested information:

Tagged release: https://github.com/TrackerControl/tracker-control-android/releases/tag/2022061501

Archive DOI: 10.5281/zenodo.6645297

Release version number: 2022061501

kasnder commented 2 years ago

Hi @sbenthall, hope you're doing well! I was wondering if there's anything else for me to do at this stage?

sbenthall commented 2 years ago

@editorialbot set 10.5281/zenodo.6645297 as archive