Pass custom form data to SCMTool.check_repository().

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

chipx86
Review Board
release-4.0.x
reviewboard

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
Pass custom form data to SCMTool.check_repository().
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)
     
     

    Could do :term:Local Site

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

    Add unused

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

    Could do :term:Local Site

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

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

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

    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)
     
     

    Same Local Site comment as above.

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

    Same Local site comment as above.

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

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