Port DiffViewerPage and DiffViewerPageView to spina.

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

david
Review Board
release-6.x
reviewboard

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
Port DiffViewerPage and DiffViewerPageView to spina.
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. Typo: "omdified" -> "modified"

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

    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. This should have a type.

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

    Missing docs.

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

  7. 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)
     
     
     
     
     
     
     
     
     

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

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

    1. View initialize just takes the one options argument.

  10. 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)
     
     
     
     
     
     
     
     
     
     

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

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

  12. Can we move to one type per line?

  13. 
      
david
chipx86
  1. 
      
  2. Second sentence must be in the description body.

  3. Summary should be on the next line.

  4. 
      
david
david
chipx86
  1. Ship It!
  2. 
      
david
Review request changed

Status: Closed (submitted)

Change Summary:

Pushed to release-6.x (222f92c)
Loading...