[1.0.x] Rover theme

Thank you for applying!

Please read Review process for security advisory coverage: What to expect → for more details and Security advisory coverage application checklist → to understand what reviewers look for. Tips for ensuring a smooth review → gives some hints for a smoother review.

The important notes are the following.

• If you have not done it yet, you should run phpcs --standard=Drupal,>DrupalPractice on the project, which alone fixes most of what reviewers would report.
• For the time this application is open, only your commits are allowed.
• The purpose of this application is giving you a new drupal.org role that allows you to opt projects into security advisory coverage, either projects you already created, or projects you will create. The project status will not be changed by this application; no other user will be able to opt projects into security advisory policy.
• We only accept an application per user. If you change your mind about the project to use for this application, or it is necessary to use a different project for the application, please update the issue summary with the link to the correct project and the issue title with the branch to review and the project name.

To the reviewers

Please read How to review security advisory coverage applications → , Application workflow → , What to cover in an application review → , and Tools to use for reviews → .

The important notes are the following.

• It is preferable to wait for a Code Review Administrator before commenting on newly created applications, even to leave a comment similar to the following one. Code Review Administrators will do some preliminary checks that are necessary before any change on the project files is suggested.
• It is also preferable to wait before using a CLI tool → to report what needs to be changed, especially because the comment left from Code Review Administrators suggests to use PHP_CodeSniffer. Before that, manual reviews should be done.
• Reviewers should not copy-paste the output of a CLI tool. They should use a CLI tool only once per application. When they do that, they should later verify the code has been correctly changed; this means, for example, that adding a documentation comment that is not correct just to avoid to get a warning/error is not a correct change that should be reported in a further comment.
• It may be best to have the applicant fix things before further review.

For new reviewers, I would also suggest to first read In which way the issue queue for coverage applications is different from other project queues → .

• What follows is a quick review of the project; it doesn't mean to be complete
• For each point, the review usually shows some lines that should be fixed (except in the case the point is about the full content of a file); it doesn't show all the lines that need to be changed for the same reason
• A review is about code that doesn't follow the coding standards, contains possible security issue, or doesn't correctly use the Drupal API; the single points aren't ordered, not even by importance

theme-settings.php

/**
 * @file
 * Implements().
 */

use Drupal\Core\Form\FormStateInterface;

/**
 * @file
 * Harmony_haven theme file.
 */

The @file tag is only used once per file. It must be used in the first comment after the <?php tag; its description, for that file, must be Functions to support [theme name] theme settings. where [theme name] is the name given in the .info.yml file.

  // dd($form);
  if ($form['#attributes']['class'][0] == 'system-theme-settings') {
    $form['#attached']['library'][] = 'rover/theme.setting';

    // Verticle tabs.
    $form['rover_tab'] = [
      '#type' => 'vertical_tabs',
      '#prefix' => '<h2><small>' . t('Rover settings') . '</small></h2>',
      '#weight' => -10,
    ];

    // Layout.
    $form['layout'] = [
      '#type' => 'details',
      '#title' => t('Layout'),
      '#group' => 'rover_tab',
    ];
    // Container.
    $form['layout']['container'] = [
      '#type' => 'details',
      '#title' => t('Container'),
      '#collapsible' => TRUE,
      '#collapsed' => TRUE,
    ];

Debugging code must be removed, not commented out. Comments should be used for something that is not clear from the code, not to repeat what the code makes already clear. Commenting with Container. the code that adds a form element titled Container is not necessary.
Furthermore, if the form element title is used for a form element and a form element contained in that, the title is ambiguous and should be changed.

'#description' => t('Use <code>.container-XX class. See @bootstrap_fluid_containers_link.', []),

I guess that @bootstrap_fluid_containers_link should be a link (otherwise, that sentence would not be understood from who read it). In that case, the markup for links is different.

    $form['banner']['vt_banner'] = [
      '#type' => 'details',
      '#title' => t('Front Page Banner'),
      '#collapsible' => TRUE,
      '#collapsed' => TRUE,
    ];

A details element uses #open, not #collapsed nor #collapsible.

/js/main.js

(function ($, Drupal, once) {
    Drupal.behaviors.roverBehavior = {
      attach: function (context, settings) {
        once('roverBehavior', 'input.roverBehavior', context).forEach(function (element) {
          // Apply the myCustomBehaviour effect to the elements only once.
        });
      }
    };
})(jQuery, Drupal, once);

We expect the code for a theme/module to be fully written; that includes JavaScript code. There should not be code that adds a JavaScript behaviour that is still to be written.

templates/form/form.html.twig

<form{{ attributes }}>
  {{ children }}
</form>

That is the default template content used by Drupal core. There is no need to override a template to use the same content used by the default template. (See core/modules/system/templates/form.html.twig.)

The same holds true for other template files, including templates/navigation/menu.html.twig.

I am changing priority as per Issue priorities → .

This thread has been idle, in the Needs work state with no activity for several months. Therefore, I am assuming that you are no longer pursuing this application, and I marked it as Closed (won't fix).

If this is incorrect, and you are still pursuing this application, then please feel free to re-open it and set the issue status to Needs work or Needs review, depending on the current status of your code.

Hello there, Apologies for the delay. I wasn’t able to complete this task earlier,
but I’ve now added the new release: 2.0.2
Repo : https://git.drupalcode.org/project/rover/

Please let me know if there's anything else I can assist with.

This is the application created by gautam_105@.
Only gautam_105@ can continue this application, which is not an application for which every maintainer of the used project can contribute. These applications are per user, not per project.

Hello, I have reviewed the code, and all the necessary changes have been made and ensure the code follows Drupal's security standards and best practices.
Please let me know if you have any more suggestions or if there's anything else I should keep in mind while making these updates. Thank you !

In the past 12 months, other maintainers have done commits. In the first comment I posted in this issue, I wrote:

• If you have not done it yet, you should run phpcs --standard=Drupal,>DrupalPractice on the project, which alone fixes most of what reviewers would report.
• For the time this application is open, only your commits are allowed.

The purpose of these applications is reviewing a project to understand what the person who applies understands about writing secure code which follows the Drupal coding standards and correctly uses the Drupal API, not what all the project maintainers collectively understand about those points.

This application can only continue with another project where the commits are done by gautam_105@.

I think it would be better to close this application for now, as some commits belong to the maintainer and creator. However, all commits should be made by gautam_105@. It would be simpler to create a new application instead.

I'm not sure I fully understand. Could you please clarify? Should I delete this application and raise a new request from a maintainer or creator account, then make all the commits from that account? I'd really appreciate your suggestions on the best way to proceed.

@gautam_105@ You are not maintainer of [1.0.x] Rover. So you can not apply for this project. You can continue this application by opting for other project. Please go through comment #1 & #11

For the time this application is open, only your commits are allowed.

Can you please check now, as I am currently the maintainer of the [1.0.x] Rover.

Still, there are no commits from gautam_105@, nor there is a branch with commits just from gautam_105@.

I will repeat it: These applications are not for projects, but for people (one person per application) who need the permission to opt projects into security advisory coverage.
They are not projects for all the project maintainers. As such, we do not review what the projects maintainers as group understand about writing secure code that follows the Drupal coding standards and correctly uses the Drupal API.

If gautam_105@ wants to be able to opt projects into security advisory policy, he needs to provide a project where all the commits have been done by him.