• 
      

    Comment serialization cleanup part 1: Break out basic definition.

    Review Request #13655 — Created March 21, 2024 and submitted

    Information

    Review Board
    release-7.x

    Reviewers

    The way comments are serialized for use in Review UIs and the diff
    viewer is extremely messy. This change is the first step of cleaning
    that up.

    When I first converted the comment models to TypeScript, I mistakenly
    marked the comment editor and comment dialog view's publishedComments
    as being BaseComment[]. These are in fact the serialized comment data
    arrays. This change moves the definition of the serialized comment
    interface out of abstractCommentBlockModel.ts into its own file, where
    we can import it anywhere we need, and makes use of that definition
    where appropriate.

    Ran js-tests.

    Summary ID
    Comment serialization cleanup part 1: Break out basic definition.
    The way comments are serialized for use in Review UIs and the diff viewer is extremely messy. This change is the first step of cleaning that up. When I first converted the comment models to TypeScript, I mistakenly marked the comment editor and comment dialog view's `publishedComments` as being `BaseComment[]`. These are in fact the serialized comment data arrays. This change moves the definition of the serialized comment interface out of `abstractCommentBlockModel.ts` into its own file, where we can import it anywhere we need, and makes use of that definition where appropriate. Testing Done: Ran js-tests.
    de136a2ca4f428f967610da03047f135c16ac36c
    Description From Last Updated

    This should probably be import type. Same applies to other serialized type imports in the change.

    chipx86chipx86

    Just to throw out the question here, do we want to alphabetize by imported name, or group type separately from …

    chipx86chipx86
    david
    chipx86
    1. 
        
    2. Show all issues

      This should probably be import type.

      Same applies to other serialized type imports in the change.

    3. 
        
    david
    maubin
    1. Ship It!
    2. 
        
    chipx86
    1. 
        
    2. Show all issues

      Just to throw out the question here, do we want to alphabetize by imported name, or group type separately from non-type within the list? I kind of like the idea of keeping them in separate groups within the same import list, so it's very clear where things go and each grouping can be sorted independently.

      1. We can group them. I'll update /r/13713/ to do that everywhere.

    3. 
        
    david
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-7.x (85bb0dc)