Comments

renatog created an issue. See original summary.

RenatoG’s picture

Assigned: RenatoG » Unassigned
Status: Active » Needs review
FileSize
66.14 KB

Patch submitted with the fixes.

Thanks.

Regards.

cebasqueira’s picture

RenatoG’s picture

Thank you very much @cebasqueira.

Fixed variable unused.

Regards.

cebasqueira’s picture

Status: Needs review » Reviewed & tested by the community

Thanks @renatog, works fine... RTBC

Berdir’s picture

Status: Reviewed & tested by the community » Needs work

Thanks, but this adds a lot of strange comments just to technically comply to the standards, can't accept that, see below.

  1. +++ b/pathauto.js
    @@ -1,3 +1,8 @@
    +/**
    + * @file
    + * Name: pathauto.js.
    + */
    +
    

    this isn't very useful, an actual description would be nice.

  2. +++ b/pathauto.module
    @@ -12,7 +13,9 @@
     /**
      * @file
    - * Main file for the Pathauto module, which automatically generates aliases for content.
    + * Main file for the Pathauto module.
    + *
    + * Which automatically generates aliases for content.
    

    that isn't really proper english anymore, we oculd leave out automatically instead, as generating kind of implies automatically.

  3. +++ b/src/Form/PatternEditForm.php
    @@ -16,11 +16,15 @@ use Symfony\Component\DependencyInjection\ContainerInterface;
     
       /**
    +   * Manager.
    +   *
        * @var \Drupal\pathauto\AliasTypeManager
        */
       protected $manager;
     
       /**
    +   * Entity.
    +   *
        * @var \Drupal\pathauto\PathautoPatternInterface
        */
    

    those descriptions are pretty bogus, if we do write something, we need to have an actual description that makes sense.

  4. +++ b/src/Form/PatternEditForm.php
    @@ -70,7 +82,9 @@ class PatternEditForm extends EntityForm {
       /**
    -   * {@inheritDoc}
    +   * Build Form.
    +   *
    +   * {@inheritDoc}.
        */
       public function buildForm(array $form, FormStateInterface $form_state) {
    

    this is not valid.

  5. +++ b/src/Form/PatternEditForm.php
    @@ -198,7 +212,9 @@ class PatternEditForm extends EntityForm {
     
       /**
    -   * {@inheritDoc}
    +   * Build Entity.
    +   *
    +   * {@inheritDoc}.
        */
       public function buildEntity(array $form, FormStateInterface $form_state) {
    

    this neither.

  6. +++ b/src/Form/PatternEditForm.php
    @@ -255,7 +273,9 @@ class PatternEditForm extends EntityForm {
    -   * {@inheritDoc}
    +   * Function to save.
    +   *
    +   * {@inheritDoc}.
        */
    

    same.

  7. +++ b/src/PathautoGeneratorInterface.php
    @@ -77,8 +79,8 @@ interface PathautoGeneratorInterface {
    -   *   - force: will force updating the path
    -   *   - language: the language for which to create the alias
    +   *   - Force: will force updating the path
    +   *   - language: the language for which to create the alias.
        *
    

    this change is also not correct, force is a lower-case key.

  8. +++ b/src/PathautoPatternInterface.php
    @@ -41,6 +48,7 @@ interface PathautoPatternInterface extends ConfigEntityInterface {
        *
        * @return int
    +   *   Return weight.
    
    @@ -58,6 +66,7 @@ interface PathautoPatternInterface extends ConfigEntityInterface {
        *
        * @return \Drupal\Core\Plugin\Context\ContextInterface[]
    +   *   Return contexts.
    

    same for those things, it only makes sense to have a description if it's at least valid english

  9. +++ b/src/PathautoPatternInterface.php
    @@ -171,10 +182,11 @@ interface PathautoPatternInterface extends ConfigEntityInterface {
        *
        * @return bool
    +   *   Return flag with type boolean.
        */
    

    this doesn't make sesne.

  10. +++ b/src/PathautoWidget.php
    @@ -24,18 +24,19 @@ class PathautoWidget extends PathWidget {
         // @todo Impossible to do this in widget, use another solution
         /*
         $form['path'] += array(
    -      '#type' => 'fieldset',
    -      '#title' => $this->t('URL path settings'),
    -      '#collapsible' => TRUE,
    -      '#collapsed' => empty($form['path']['alias']),
    -      '#group' => 'additional_settings',
    -      '#attributes' => array(
    -        'class' => array('path-form'),
    -      ),
    -      '#access' => \Drupal::currentUser()->hasPermission('create url aliases') || \Drupal::currentUser()->hasPermission('administer url aliases'),
    -      '#weight' => 30,
    -      '#tree' => TRUE,
    -      '#element_validate' => array('path_form_element_validate'),
    +    '#type' => 'fieldset',
    +    '#title' => $this->t('URL path settings'),
    +    '#collapsible' => TRUE,
    +    '#collapsed' => empty($form['path']['alias']),
    +    '#group' => 'additional_settings',
    +    '#attributes' => array(
    +    'class' => array('path-form'),
    

    this change doesn't make sense, this is commented out code.

  11. +++ b/src/Tests/PathautoBulkUpdateTest.php
    @@ -54,7 +56,10 @@ class PathautoBulkUpdateTest extends WebTestBase {
     
    -  function testBulkUpdate() {
    +  /**
    +   * Function testBulkUpdate().
    +   */
    +  public function testBulkUpdate() {
    

    this and more below are also bad, if we have a docblock it needs to be an actual sentence.

hgunicamp’s picture

Continuing the work of the patch 'pathauto-codingstandards-2833680-4.patch'.

hgunicamp’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 7: coding_standards_for-2833680-7.patch, failed testing. View results

hgunicamp’s picture

Status: Needs work » Needs review
FileSize
982 bytes
157.95 KB

Sorry. A typo when saving the file.

Status: Needs review » Needs work

The last submitted patch, 10: coding_standards_for-2833680-10.patch, failed testing. View results