• 
      

    Pass custom form data to SCMTool.check_repository().

    Review Request #12577 — Created Sept. 2, 2022 and submitted

    Information

    Review Board
    release-4.0.x

    Reviewers

    Review Board 3.0.16 introduced support for custom forms for SCMTools,
    storing any saved fields in Repository.extra_data. However, this data
    was never passed to check_repository(), meaning that validation could
    not happen based on this data for plain SCMTools (though it was passed
    to hosting services).

    This change adds support for passing custom data to
    check_repository(). This requires that the implementation supports
    **kwargs (a deprecation warning will be emitted if not) and accepts
    all arguments as keyword arguments in any order. This will be required
    in Review Board 5. All custom form data will then be passed.

    In the process, some of the RepositoryForm code has been cleaned up.
    There was a _build_repository_extra_data() method that only worked for
    hosting services, and was mostly made redundant when per-SCM subforms
    were added. It's now been removed, utilizing the subform data instead.

    Unit tests pass on Python 2 and 3.

    Tested creating new GitHub repositories, verifying that hosting service
    forms still worked correctly.

    Tested creating new standard Git repositories, with and without the
    updates to check_repository().

    Tested creating a new repository for SOS making use of form data in
    check_repository().

    Summary ID
    Pass custom form data to SCMTool.check_repository().
    Review Board 3.0.16 introduced support for custom forms for SCMTools, storing any saved fields in `Repository.extra_data`. However, this data was never passed to `check_repository()`, meaning that validation could not happen based on this data for plain SCMTools (though it was passed to hosting services). This change adds support for passing custom data to `check_repository()`. This requires that the implementation supports `**kwargs` (a deprecation warning will be emitted if not) and accepts all arguments as keyword arguments in any order. This will be required in Review Board 5. All custom form data will then be passed. In the process, some of the `RepositoryForm` code has been cleaned up. There was a `_build_repository_extra_data()` method that only worked for hosting services, and was mostly made redundant when per-SCM subforms were added. It's now been removed, utilizing the subform data instead.
    4e7a46e02dda5a7c357ffe233b42a82366c9d017
    Description From Last Updated

    Could do :term:Local Site

    maubinmaubin

    Add unused

    maubinmaubin

    Could do :term:Local Site

    maubinmaubin

    Should we add a Version Changed: to the class too?

    maubinmaubin

    Should document the KeyError.

    maubinmaubin

    Same Local Site comment as above.

    maubinmaubin

    Same Local site comment as above.

    maubinmaubin

    Could update this to be as descriptive as the other descriptions.

    maubinmaubin
    maubin
    1. 
        
    2. reviewboard/scmtools/core.py (Diff revision 1)
       
       
      Show all issues

      Could do :term:Local Site

    3. reviewboard/scmtools/core.py (Diff revision 1)
       
       
      Show all issues

      Add unused

    4. reviewboard/scmtools/cvs.py (Diff revision 1)
       
       
      Show all issues

      Could do :term:Local Site

    5. reviewboard/scmtools/errors.py (Diff revision 1)
       
       
      Show all issues

      Should we add a Version Changed: to the class too?

    6. reviewboard/scmtools/forms.py (Diff revision 1)
       
       
      Show all issues

      Should document the KeyError.

      1. Which?

      2. Woops, I meant the ValidationError that's raised when we catch KeyError's.

      3. Ahh okay, I hoped that's what you meant!

        I can't wait to eventually clean up this whole form.

    7. reviewboard/scmtools/git.py (Diff revision 1)
       
       
      Show all issues

      Same Local Site comment as above.

    8. reviewboard/scmtools/hg.py (Diff revision 1)
       
       
      Show all issues

      Same Local site comment as above.

    9. reviewboard/scmtools/perforce.py (Diff revision 1)
       
       
       
      Show all issues

      Could update this to be as descriptive as the other descriptions.

    10. 
        
    chipx86
    maubin
    1. Ship It!
    2. 
        
    david
    1. Ship It!
    2. 
        
    chipx86
    Review request changed
    Status:
    Completed
    Change Summary:
    Pushed to release-4.0.x (f97a289)