Port DiffViewerPage and DiffViewerPageView to spina.

Review Request #12940 — Created April 5, 2023 and submitted

Information

Review Board
release-6.x

Reviewers

This change ports the diffviewer page model and view to spina. These
files have been moved into the reviews subdirectory to correspond to
the bundle they're a part of.

  • Ran js-tests.
  • Tested the diffviewer page.
Summary ID
Port DiffViewerPage and DiffViewerPageView to spina.
This change ports the diffviewer page model and view to spina. These files have been moved into the `reviews` subdirectory to correspond to the bundle they're a part of. Testing Done: - Ran js-tests. - Tested the diffviewer page.
cbd21aaa36381889c43c6323def62bb5df33efa0
Description From Last Updated

Typo: "omdified" -> "modified"

chipx86chipx86

Do we want to document the incoming data to be parsed? I feel it's still useful, maybe to list what …

chipx86chipx86

Second sentence must be in the description body.

chipx86chipx86

This should have a type.

chipx86chipx86

Summary should be on the next line.

chipx86chipx86

Missing docs.

chipx86chipx86

We should probably just accept ...args: any[] and pass that in super.initialize().

chipx86chipx86

Since there are multiple types involved, can we put the param and the return type on their own lines?

chipx86chipx86

Probably worth making this an interface, so we can reference it elsewhere.

chipx86chipx86

Should we accept/pass ...args: any[]?

chipx86chipx86

I know we already had this, but we should probably do this last (and just return the result) so we …

chipx86chipx86

Let's add an interface for this. We can then move the documentation into that.

chipx86chipx86

Maybe here too?

chipx86chipx86

Can we move to one type per line?

chipx86chipx86
david
chipx86
  1. 
      
  2. Show all issues

    Typo: "omdified" -> "modified"

  3. reviewboard/static/rb/js/reviews/models/diffViewerPageModel.ts (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Do we want to document the incoming data to be parsed? I feel it's still useful, maybe to list what they'd map to.

    1. Hmm, I'm not sure. Maybe not yet, but I'll revisit the idea as more of this gets fleshed out.

  4. Show all issues

    This should have a type.

  5. reviewboard/static/rb/js/reviews/models/diffViewerPageModel.ts (Diff revision 2)
     
     
     
     
     
     
     
     
    Show all issues

    Missing docs.

  6. Show all issues

    We should probably just accept ...args: any[] and pass that in super.initialize().

  7. Show all issues

    Since there are multiple types involved, can we put the param and the return type on their own lines?

  8. reviewboard/static/rb/js/reviews/models/diffViewerPageModel.ts (Diff revision 2)
     
     
     
     
     
     
     
     
     
    Show all issues

    Probably worth making this an interface, so we can reference it elsewhere.

  9. Show all issues

    Should we accept/pass ...args: any[]?

    1. View initialize just takes the one options argument.

  10. Show all issues

    I know we already had this, but we should probably do this last (and just return the result) so we can clear up sub-views before this view's element goes away.

  11. reviewboard/static/rb/js/reviews/views/diffViewerPageView.ts (Diff revision 2)
     
     
     
     
     
     
     
     
     
     
    Show all issues

    Let's add an interface for this. We can then move the documentation into that.

  12. Show all issues

    Maybe here too?

    1. This is so small that I don't think it's worth it (yet)

  13. Show all issues

    Can we move to one type per line?

  14. 
      
david
chipx86
  1. 
      
  2. Show all issues

    Second sentence must be in the description body.

  3. Show all issues

    Summary should be on the next line.

  4. 
      
david
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed
Status:
Completed
Change Summary:
Pushed to release-6.x (222f92c)