Closed (fixed)
Project:
Drupal.org security advisory coverage applications
Component:
module
Priority:
Normal
Category:
Task
Assigned:
Reporter:
Created:
18 Feb 2022 at 05:16 UTC
Updated:
27 Jul 2022 at 10:24 UTC
Jump to comment: Most recent
Comments
Comment #2
hemant bansal commentedComment #3
arthur.baghdasar commentedPlease clean up the code. I see some commented code in the .module file. Also I don't see docblocks in the files for example ( @file docblock in the class).
Remove .install file content and move it to schema file in the config directory. See example here https://git.drupalcode.org/project/devel/-/blob/4.x/config/schema/devel....
Comment #4
hemant bansal commentedThanks Arthur for reviewing this. I have update all the suggested changes.
Comment #5
avpadernoComment #6
avpadernoClasses that extend
ConfigFormBasecan use$this->config().There is no need to use a prefix for the configuration names. The configuration object name is already unique for the module.
Files available in other repositories must not be copied into drupal.org repositories, especially if they contain code licensed under a license that isn't the one use from Drupal core, such as jquery.scrollUp.js and jquery.scrollUp.min.js.
Comment #7
rohitrajputsahab commented@Hemant Bansal
These types of modules are already available in Drupal.org
EG:-
https://www.drupal.org/project/scrollup
https://www.drupal.org/project/jquery_scrollup
https://www.drupal.org/project/back_to_top
Can you please tell us what is the difference between your module and these modules? Why should we use the "[D9] Scroll to Top Button" module?
Please add in the description "Similar projects" and How they are different from your module?
Comment #8
hemant bansal commentedThanks @Alberto Paderno, I have made the changes as suggested. Thanks for the review.
Comment #9
geoanders commentedRan a quick standards scan. See results below:
Comment #10
avpadernoComment #11
avpadernoComment #12
hemant bansal commentedComment #13
hemant bansal commentedThanks Everyone for the review. I have made the changes as suggested.
Comment #14
hemant bansal commentedComment #15
avpadernoAs for comment #7:
I would say the project page should document the difference between this module and the existing ones.
Comment #16
hemant bansal commentedI have updated the project description as suggested. Thanks again for reviewing.
Comment #17
rohitrajputsahab commented@Hemant Bansal
Please fix the below errors and warnings.
FILE: ...\scroll_top_button\config\install\scroll_top_button.settings.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
8 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\css\scroll.top.button.css
----------------------------------------------------------------------
FOUND 91 ERRORS AFFECTING 78 LINES
----------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n"
| | but found "\r\n"
3 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
4 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
5 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
6 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
7 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
9 | ERROR | [x] Multiple selectors should each be on a single line
9 | ERROR | [x] Multiple selectors should each be on a single line
9 | ERROR | [x] Whitespace found at end of line
10 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
11 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
16 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
17 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
18 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
19 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
20 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
21 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
22 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
23 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
24 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
25 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
26 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
27 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
28 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
29 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
30 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
30 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #e6e6e6 but found #E6E6E6
31 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
31 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #ebebeb but found #EBEBEB
31 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #dedede but found #DEDEDE
32 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
32 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #ebebeb but found #EBEBEB
32 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #dedede but found #DEDEDE
33 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
33 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #ebebeb but found #EBEBEB
33 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #dedede but found #DEDEDE
34 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
34 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #ebebeb but found #EBEBEB
34 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #dedede but found #DEDEDE
35 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
35 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #ebebeb but found #EBEBEB
35 | ERROR | [x] CSS colours must be defined in lowercase; expected
| | #dedede but found #DEDEDE
36 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
37 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
38 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
39 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
40 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
44 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
49 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
50 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
51 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
52 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
53 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
54 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
55 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
56 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
57 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
58 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
59 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
60 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
61 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
62 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
63 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
64 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
65 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
69 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
74 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
75 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
76 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
77 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
78 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
83 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
84 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
85 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
86 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
87 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
88 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
89 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
90 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
91 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
92 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
93 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
94 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
95 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
96 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
97 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
98 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
99 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
103 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
108 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
109 | ERROR | [x] Line indented incorrectly; expected 2 spaces,
| | found 4
----------------------------------------------------------------------
PHPCBF CAN FIX THE 91 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\js\scroll.top.button.js
----------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 10 LINES
----------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but
| | found "\r\n"
5 | ERROR | [x] Whitespace found at end of line
12 | ERROR | [x] Functions must not contain multiple empty lines in
| | a row; found 2 empty lines
13 | ERROR | [x] Whitespace found at end of line
16 | ERROR | [x] Whitespace found at end of line
25 | ERROR | [x] Whitespace found at end of line
39 | ERROR | [x] Whitespace found at end of line
66 | ERROR | [x] Whitespace found at end of line
69 | ERROR | [x] Whitespace found at end of line
70 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 10 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\README.md
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
29 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\scroll_top_button.info.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
7 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\scroll_top_button.libraries.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
9 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\scroll_top_button.links.menu.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
5 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\scroll_top_button.module
----------------------------------------------------------------------
FOUND 36 ERRORS AFFECTING 21 LINES
----------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but
| | found "\r\n"
30 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found
| | 4
31 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found
| | 4
31 | ERROR | [x] No space found before comment text; expected "//
| | check if scroll to top button is enabled or
| | disabled" but found "//check if scroll to top
| | button is enabled or disabled"
31 | ERROR | [x] Inline comments must start with a capital letter
31 | ERROR | [x] Inline comments must end in full-stops, exclamation
| | marks, question marks, colons, or closing
| | parentheses
32 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found
| | 4
33 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
| | 6
33 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "TRUE" but found "true"
34 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
| | 6
34 | ERROR | [x] No space found before comment text; expected "//
| | check for the admin routes/pages" but found
| | "//check for the admin routes/pages"
34 | ERROR | [x] Inline comments must start with a capital letter
34 | ERROR | [x] Inline comments must end in full-stops, exclamation
| | marks, question marks, colons, or closing
| | parentheses
35 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
| | 6
36 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
36 | ERROR | [x] TRUE, FALSE and NULL must be uppercase; expected
| | "FALSE" but found "false"
37 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
| | 6
39 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
| | 6
40 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
40 | ERROR | [x] No space found before comment text; expected "//
| | attach scroll to top button library" but found
| | "//attach scroll to top button library"
40 | ERROR | [x] Inline comments must start with a capital letter
40 | ERROR | [x] Inline comments must end in full-stops, exclamation
| | marks, question marks, colons, or closing
| | parentheses
41 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
42 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
42 | ERROR | [x] No space found before comment text; expected "//
| | set configuration values to drupal settings" but
| | found "//set configuration values to drupal
| | settings"
42 | ERROR | [x] Inline comments must start with a capital letter
42 | ERROR | [x] Inline comments must end in full-stops, exclamation
| | marks, question marks, colons, or closing
| | parentheses
43 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
44 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
45 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
46 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
47 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
48 | ERROR | [x] Line indented incorrectly; expected 6 spaces, found
| | 8
48 | ERROR | [x] Whitespace found at end of line
49 | ERROR | [x] Line indented incorrectly; expected 4 spaces, found
| | 6
50 | ERROR | [x] Line indented incorrectly; expected 2 spaces, found
| | 4
----------------------------------------------------------------------
PHPCBF CAN FIX THE 36 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\scroll_top_button.routing.yml
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
7 | ERROR | [x] Expected 1 newline at end of file; 0 found
----------------------------------------------------------------------
PHPCBF CAN FIX THE 1 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
FILE: D:\scroll_top_button\src\Form\ScrollTopButtonSettingsForm.php
----------------------------------------------------------------------
FOUND 2 ERRORS AFFECTING 2 LINES
----------------------------------------------------------------------
1 | ERROR | [x] End of line character is invalid; expected "\n" but
| | found "\r\n"
3 | ERROR | [x] Namespaced classes, interfaces and traits should not
| | begin with a file doc comment
----------------------------------------------------------------------
PHPCBF CAN FIX THE 2 MARKED SNIFF VIOLATIONS AUTOMATICALLY
----------------------------------------------------------------------
Time: 680ms; Memory: 10MB
Comment #18
hemant bansal commentedComment #19
avpadernoPlease remember that, until this application isn't closed, the branch used for this application must contain commits only from the user who applied.
Comment #20
hemant bansal commentedThanks @apaderno, I will take care of the commits in future.
@rohit-rajput-sahab , Can you please make sure you did the coding stranded check on latest branch '1.2.x'. As I have checked again and found nothing.
Comment #21
LuongGiap commentedHi @Hemant Bansal, I reviewed coding standards, issue was fixed on branch '1.2.x'
Comment #22
avpadernoThank 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 IRC #drupal-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 dedicated reviewers.
Comment #23
hemant bansal commentedThanks @apaderno, @LuongGiap for reviewing along with suggestions those will be helpful for us to do more contributions.