Drupal Dependency Issues [#2895373]

There some Drupal Dependency issues in this module.

FILE: /root/repos/pareviewsh/pareview_temp/config_export_ignore.info.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
7 | WARNING | All dependencies must be prefixed with the project name,
| | for example "drupal:"
--------------------------------------------------------------------------

FILE: ...root/repos/pareviewsh/pareview_temp/config_export_ignore.routing.yml
--------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------
8 | WARNING | The administration page callback should probably use
| | "administer site configuration" - which implies the user
| | can change something - rather than "access administration
| | pages" which is about viewing but not changing
| | configurations.
--------------------------------------------------------------------------

Comment	File	Size	Author
#8	config_export_ignore_coding_standard-2895373-8.patch	3.9 KB	jayesh_makwana
#4	config_export_ignore_coding_standard-2895373-4.patch	2.55 KB	jayesh_makwana
#2	config_export_ignore_drupal_dependency-2895373-2.patch	838 bytes	jayesh_makwana

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

17 July 2017 at 14:42

jayesh_makwana created an issue. See original summary.

Comment #2

jayesh_makwana CreditAttribution: jayesh_makwana at cmsMinds commented 17 July 2017 at 14:43

File	Size
config_export_ignore_drupal_dependency-2895373-2.patch	838 bytes

We created a patch for solving Drupal Dependency issues for this module. Please check and apply it.

Comment #3

mohit_aghera CreditAttribution: mohit_aghera as a volunteer and at Axelerant commented 18 July 2017 at 06:41

Status:

Active

» Needs work

Hi @jayesh_makwana
Thank you for doing code review.
I checked on pareview.sh, there are few more coding standards issues.
It would be great if you can include that as well
See here https://pareview.sh/node/2406
If it's not accessible you can run it again to see result.

Comment #4

jayesh_makwana CreditAttribution: jayesh_makwana at cmsMinds commented 18 July 2017 at 07:38

File	Size
config_export_ignore_coding_standard-2895373-4.patch	2.55 KB

Hello mohit_aghera,

Thanks for accepting my patch. Actually, I show https://pareview.sh/node/2406 and review those issues. Basically, I solved some issues and created a patch. But, I don't know about some issues like below:

FILE: ...onfig_export_ignore/src/ConfigExportIgnoreConfigSplitService.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 2 WARNINGS AFFECTING 2 LINES
----------------------------------------------------------------------
30 | WARNING | Possible useless method overriding detected
101 | WARNING | The use of function fnmatch() is discouraged
----------------------------------------------------------------------

FILE: ...alwebcopy/modules/config_export_ignore/src/Form/SettingsForm.php
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
47 | WARNING | Possible useless method overriding detected
----------------------------------------------------------------------

Those methods are used for this module or not which I don't know. So please check.

Comment #5

jayesh_makwana CreditAttribution: jayesh_makwana at cmsMinds commented 18 July 2017 at 08:41

Status:

Needs work

» Needs review

Comment #6

mohit_aghera CreditAttribution: mohit_aghera as a volunteer and at Axelerant commented 19 July 2017 at 03:54

Hi
Ignore the issues in FILE: ...config_export_ignore/src/ConfigExportIgnoreConfigSplitService.php for now.

For file config_export_ignore/src/Form/SettingsForm.php remove the validate function in class, as we are not doing anything over there.
Moreover, in both patches you have added separate changes, can you combine all those changes into one and create single patch.