citp / news-disinformation-study

A research project on how web users consume, are exposed to, and share news online.
8 stars 2 forks source link

[DO NOT MERGE] Rally engineering review 1 #94

Closed rhelmer closed 3 years ago

rhelmer commented 3 years ago

This is a PR for review purposes. Unfortunately, we can't merge it but it gives us a transparent place to discuss the whole repository. Any changes we decide to make should be done in separate PR(s), and this can be closed when the review is concluded.

rhelmer commented 3 years ago

@akohlbre sorry this PR is all strange and backward, this is the best way I know to do a review of an existing codebase :)

As I mentioned above I'm not super concerned about any of the outstanding issues in WebScience as we are addressing these upstream, but I am concerned about not being able to merge in any upstream changes. I think what's here is good enough for add-on review, but I'm not still not sure we're doing ourselves any favors by using this instead of the packaged WebScience repo.

I'm trying to set the quality bar at:

. The only concern I have right now is with the latter due to the WebScience fork :)

rhelmer commented 3 years ago

The Princeton team decided to merge the ws-migration branch (moving over to use the packaged @mozilla/web-science), so closing this review.