Problem/Motivation

A code review of this module turned up a few packaging and test-coverage items worth addressing. Nothing here is a functional bug; the module works correctly as shipped.

  1. Missing .gitignore | Severity: Minor | File: project root

    There is no .gitignore file at all. At minimum it should ignore .DS_Store (and any local IDE files).
  2. composer.json missing an "authors" section | Severity: Minor | File: composer.json

    composer.json has "support" (issues/source) but no "authors" entry listing the maintainer(s) with name, drupal.org profile URL and role, which is the convention documented in drupal.org packaging guidelines.
  3. Missing test coverage for the broken Media type fallback path | Severity: Minor | File: src/Hook/RemoteVideoShapeMatchingHooks.php

    getSourceFieldDefinition() catches PluginException/InvalidArgumentException/LogicException and logs a warning so a broken Media type does not break shape matching site-wide, but RemoteVideoShapeMatchingTest.php has no test exercising that catch branch (e.g. a Media type whose source field was removed after creation). Worth adding a kernel test for it since it is the one piece of defensive logic in the module.

Proposed resolution

  • Add a .gitignore with .DS_Store.
  • Add an "authors" section to composer.json.
  • Add a kernel test covering the exception-handling branch in getSourceFieldDefinition().

AI was used to investigate and for better solutions. The issues were then manually reviewed/tested.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

zeeshan_khan created an issue. See original summary.

zeeshan_khan’s picture

Issue summary: View changes

revathi.b made their first commit to this issue’s fork.

revathi.b’s picture

Status: Active » Needs review

@zeeshan Can you please review MR !1.

  • revathi.b committed 8f19c089 on 1.0.x
    Issue #3632447: Add .gitignore, composer authors and test coverage for...
zeeshan_khan’s picture

Status: Needs review » Reviewed & tested by the community
zeeshan_khan’s picture

Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.