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.
--------------------------------------------------------------------------

CommentFileSizeAuthor
#8 config_export_ignore_coding_standard-2895373-8.patch3.9 KBjayesh_makwana
#4 config_export_ignore_coding_standard-2895373-4.patch2.55 KBjayesh_makwana
#2 config_export_ignore_drupal_dependency-2895373-2.patch838 bytesjayesh_makwana

Comments

jayesh_makwana created an issue. See original summary.

jayesh_makwana’s picture

jayesh_makwana commented
StatusFileSize
new config_export_ignore_drupal_dependency-2895373-2.patch838 bytes

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

mohit_aghera’s picture

mohit_aghera commented
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.

jayesh_makwana’s picture

jayesh_makwana commented
StatusFileSize
new config_export_ignore_coding_standard-2895373-4.patch2.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.

jayesh_makwana’s picture

jayesh_makwana commented
Status: Needs work » Needs review
mohit_aghera’s picture

mohit_aghera commented

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.

mohit_aghera’s picture

mohit_aghera commented
Status: Needs review » Needs work
jayesh_makwana’s picture

jayesh_makwana commented
StatusFileSize
new config_export_ignore_coding_standard-2895373-8.patch3.9 KB

Hello mohit_aghera,

Thanks for the feedback. I upload updated patch. Please check and apply it.

jayesh_makwana’s picture

jayesh_makwana commented
Status: Needs work » Needs review

mohit_aghera’s picture

mohit_aghera commented
Status: Needs review » Fixed

@jayesh_makwana
Thank you for your time.
Committed and fixed.

Status: Fixed » Closed (fixed)

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