This module adds a plugin to the Geocoder module in order to make DAWA (Danmarks Adressers Web API) do the geocoding.

Project Link

https://www.drupal.org/project/geocoder_dawa

Comments

blyme created an issue. See original summary.

shashank5563’s picture

Thank you for applying! Reviewers will review the project files, describing what needs to be changed.

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.

To reviewers: Please read How to review security advisory coverage applications, What to cover in an application review, and Drupal.org security advisory coverage application workflow.

While this application is open, only the user who opened the application can make commits to the project used for the application.

Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.

shashank5563’s picture

Issue summary: View changes
shashank5563’s picture

Status: Active » Needs work

main is a wrong branch name, as branch names end with the literal .x. That branch needs to be removed.

shashank5563’s picture

PHPCS issue

vendor/bin/phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml,php  /modules/geocoder_dawa/

FILE: /modules/geocoder_dawa/tests/src/Kernel/GeocoderDawaKernelTest.php
----------------------------------------------------------------------------------------------------------
FOUND 6 ERRORS AFFECTING 6 LINES
----------------------------------------------------------------------------------------------------------
  1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
  6 | ERROR | [x] There must be one blank line after the last USE statement; 2 found;
 49 | ERROR | [x] Expected 1 blank line after function; 2 found
 55 | ERROR | [ ] Description for the @return value is missing
 68 | ERROR | [ ] Description for the @return value is missing
 81 | ERROR | [ ] Description for the @return value is missing
----------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 3 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------------------------------------------


FILE: /modules/geocoder_dawa/src/Plugin/Geocoder/Provider/Dawa.php
-----------------------------------------------------------------------------------------------------------------------
FOUND 13 ERRORS AFFECTING 13 LINES
-----------------------------------------------------------------------------------------------------------------------
   1 | ERROR | [x] End of line character is invalid; expected "\n" but found "\r\n"
  34 | ERROR | [ ] Doc comment short description must be on a single line, further text should be a separate paragraph
  42 | ERROR | [x] Expected 1 space between type hint and argument "$configuration"; 26 found
  45 | ERROR | [x] Expected 1 space between type hint and argument "$config_factory"; 9 found
  46 | ERROR | [x] Expected 1 space between type hint and argument "$cache_backend"; 10 found
  47 | ERROR | [x] Expected 1 space between type hint and argument "$language_manager"; 7 found
  48 | ERROR | [x] Expected 1 space between type hint and argument "$client"; 6 found
 179 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 8
 180 | ERROR | [x] Object operator not indented correctly; expected 10 spaces but found 8
 182 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found 8
 208 | ERROR | [x] Expected newline after closing brace
 214 | ERROR | [x] Expected 1 blank line after function; 0 found
 215 | ERROR | [x] The closing brace for the class must have an empty line before it
-----------------------------------------------------------------------------------------------------------------------
PHPCBF CAN FIX THE 12 MARKED SNIFF VIOLATIONS AUTOMATICALLY
-----------------------------------------------------------------------------------------------------------------------

Time: 353ms; Memory: 12MB
avpaderno’s picture

Title: Geocoder DAWA » [1.x] Geocoder DAWA
Issue summary: View changes
blyme’s picture

Status: Needs work » Needs review

Hi

Thanks a lot for your time. I've updated with a new release 1.0.3-beta1.

I've removed the main branch and fixed the phpcs issues.

vishal.kadam’s picture

@blyme There is no need to create a release for a review. Just commit the changes to the 1.x branch.

vishal.kadam’s picture

Status: Needs review » Needs work

1. FILE: geocoder_dawa.info.yml

core_version_requirement: ^8 || ^9

The Drupal Core versions before 8.7.7 do not recognize the core_version_requirement: key.

2. FILE: src/Plugin/Geocoder/Provider/Dawa.php

  /**
   * {@inheritDoc}
   */
  public function geocode($source) {
  /**
   * {@inheritDoc}
   */
  public function reverse($latitude, $longitude) {

{@inheritdoc} is used if you are overriding or implementing a method from a base
class or interface.

avpaderno’s picture

The Dawa class inherits its methods from the ProviderBase class. For those methods, the documentation comment is correct.

It is not correct for the class constructor, as @{inheritdoc} is not used for class constructors.

blyme’s picture

Status: Needs work » Needs review

Hi again

Thank you. I've updated the 1.x branch (without a new release :D)

I've added the core: 8.x to the .info.yml file and fixed the doc comment for the constructor.

vishal.kadam’s picture

Rest looks fine to me.

Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.

avpaderno’s picture

Status: Needs review » Reviewed & tested by the community

Thank you for your contribution! I am going to update your account.

These are some recommended readings to help with excellent maintainership:

You can find more contributors chatting on the Slack #contribute channel. So, come hang out and stay involved.
Thank you, also, for your patience with the review process.
Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

I thank all the reviewers.

avpaderno’s picture

Status: Reviewed & tested by the community » Fixed
blyme’s picture

Thanks for your time. Appreciate it.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.