Closed (fixed)
Project:
YMCA Donate
Version:
2.0.3
Component:
Code
Priority:
Minor
Category:
Task
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
11 Jul 2023 at 07:41 UTC
Updated:
20 Feb 2024 at 14:59 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #2
satish_kumar commentedHello, @urvashi_vora thanks for the patch, I have applied your patch and ran successfully.
These are the steps I followed:
1. Took clone from git version 1.0.x in drupal 10.1.x
2. Ran this command:
3. Applied your patch and, again ran phpcs command
found errors
4. I have fixed that error with phpcbf.
Ran this command to fix the errors:
Then again checked with phpcs:
5. Some warnings & errors are still there.
Needs work.
Comment #3
avpadernoComment #5
aayushmankotia commentedHi,
I have fixed all the errors, please review my patch .
Comment #6
Tshewang Gyaltshen commentedhi @aayushmankotia,
i have tested your patch: issuefixed-3373804-5.patch and it is fixing all the issues. Thanks for your patch. Moving to RTBC
Comment #7
avpadernoThe markup for links is written on a single line. The correct text is the following.
The last line is not correctly indented.
The first line has been split after 65 characters.
The first line has been split after 70 characters. To reach 80 characters, you may could have been placed on the first line.
The last line is not correctly indented.
The usual description for a .install file is Install, update, and uninstall functions for the [module name] module. where [module name] is the module name reported in the .info.yml file.
Since the method returns a list of form providers, it is not a form provider.
Comment #8
nitin_lamaComment #9
nitin_lamaProviding updated patch. Please review.
Comment #10
nitin_lamaComment #11
podarokCan we have Merge Request created, please?
Patch is 27Kb of size, it is not reviewable
We are in 21st century, Merge Request is a thing
Comment #12
nitin_lamaComment #14
nitin_lamaPlease review MR.
Comment #15
nitin_lamaComment #17
nikolay shapovalov commentedComment #18
mohd sahzad commentedHI @Nikolay Shapovalov,
I have fixed phpcs isssue in above attached patch 10 , please review this patch
Comment #19
nikolay shapovalov commented@Mohd Sahzad thanks for your patch.
Let's use MR workflow for this issue,
Changes you provided doesn't make sense.
The name of the module is "Y Layout Builder - Donate", and you can split it into to sentences.
You can try to use module machine name, or you can get inpiration from MR 6
The issue summary should always describe what the issue is trying to fix and, in the case, of coding standards issues, show which command has been used, which arguments have been used, and which report that command shown.
Comment #20
avpadernoI would rather use Install, update, and uninstall functions for the Y Layout Builder - Donate module. and ignore the report about the line length.
Comment #21
nikolay shapovalov commented+1 for #20.
Comment #22
avpadernoSee also comment #11, where a project maintainer asked to use issue forks and merge requests for this issue. Comment #18 still uses a patch.
Comment #23
avpadernoComment #24
avpadernoIt is not possible to create an issue fork for the 2.0.x branch, from this issue.
Comment #25
Shreyas gowda commentedComment #26
avpadernoComment #29
anjali mehta commentedKindly Review the MR.
Thank You.
Comment #30
anjali mehta commentedComment #31
podarokComment #33
sourabhjainI have removed the CSS file. Please review it now.
Comment #34
podarok