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