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: Closed (submitted)

Change Summary:

Pushed to release-4.0.x (f97a289)
Loading...