- ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
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.
core: 8.x +core_version_requirement: ^8
There is no need to add
core_version_requirement: ^8
. - Status changed to Needs review
over 1 year ago 9:17am 13 March 2023 - ๐ฎ๐ณIndia Akram Khan Cuttack, Odisha
added updated patch it fixed all phpcs issues and address #13
- Status changed to Needs work
over 1 year ago 5:16pm 13 March 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
The status is for the issue summary, which is yet to be updated.
- Status changed to Needs review
over 1 year ago 5:44pm 13 March 2023 - ๐ฎ๐ณIndia sahil.goyal
- Status changed to Needs work
over 1 year ago 7:32pm 13 March 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
/** - * Class RegisterRedirect. + * Class of RegisterRedirect. */
That comment is still just saying the class name. Adding a preposition does not make any difference; it still does not describe the class purpose.
+ * Configuration Factory.
+ *
+ * @var \Drupal\Core\Config\ConfigFactoryThe phrase need an article. Factory does not need to be capitalized, since it is not at the beginning of the sentence.
+ /** + * Current User. + * + * @var \Drupal\Core\Session\AccountProxyInterface
The same is true for this description, in this case for User.
+ /** + * RegisterSubscriber constructor.
The class name does not include the namespace. Descriptions for methods, like the descriptions for functions, start with a verb that uses the present tense and the third person singular.
+ * @param \Drupal\Core\Session\AccountProxyInterface $accountProxy + * The current user service.
If that is described as service, then it is the account proxy service.
- Status changed to Needs review
over 1 year ago 8:09am 16 March 2023 - Status changed to Needs work
over 1 year ago 8:53am 16 March 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
- * Karthik Kumar D K (heykarthikwithu) - https://www.drupal.org/u/heykarthikwithu + * Karthik Kumar D K (heykarthikwithu)- https://www.drupal.org/u/heykarthikwithu
The existing line is already correctly formatted, contrarily to the changed line, which removes a space where it should not be removed.
/** - * Class RegisterRedirect. + * Defines a generic controller to registration redirect message. *
/
What follows to is usually a verb, not a noun.
+ /** + * Constructs a new RegisterSubscriber instance. + * + * @param \Drupal\Core\Config\ConfigFactory $configFactory + * The config factory service. + * @param \Drupal\Core\Session\AccountProxyInterface $current_user + * The current user service.
That doesn't fix what reported in the previous comment, neither for the namespace nor for the service description.
- Assigned to imustakim
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 12:02pm 16 March 2023 - Status changed to Needs work
over 1 year ago 2:09pm 16 March 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
- * Karthik Kumar D K (heykarthikwithu) - https://www.drupal.org/u/heykarthikwithu + * Karthik Kumar DK (heykarthikwithu) - https://www.drupal.org/u/heykarthikwithu
The existing line is already correct, since that is what the user profile shows.
- * Class RegisterRedirect. + * Defines a generic controller to register redirect message.
It should be to show a message on redirect, since it does not register messages nor message handlers.รน
+ /** + * Constructs a new RegisterSubscriber instance. + *
The namespace is missing.
- Assigned to imustakim
- ๐ฎ๐ณIndia imustakim Ahmedabad
- * Karthik Kumar D K (heykarthikwithu) - https://www.drupal.org/u/heykarthikwithu + * Karthik Kumar DK (heykarthikwithu) - https://www.drupal.org/u/heykarthikwithu
This line exceeds 80 character and gives error in phpcs. Its only one space that is causing the issue.
+ /** + * Constructs a new \Drupal\registration_invite\EventSubscriber\RegisterSubscriber instance. + *
Adding namespace also exceeds the 80 character and gives error in phpcs.
Added changes and updated the patch.
Please review. - Issue was unassigned.
- Status changed to Needs review
over 1 year ago 11:41am 21 March 2023 - Status changed to Needs work
over 1 year ago 12:02pm 21 March 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
Yet, the name shown in the user profile is Karthik Kumar D K, not Karthik Kumar DK. Just ignore that error, or find another way to fix it.
Between showing the class namespace and having a line that is shorted than 81 characters, it is more important to show the class namespace. To short the sentence is sufficient to avoid new and use object instead of instance.
- Assigned to imustakim
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 1:33pm 2 May 2023 - Status changed to Needs work
over 1 year ago 2:59pm 2 May 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
- * Karthik Kumar D K (heykarthikwithu) - https://www.drupal.org/u/heykarthikwithu + * Karthik Kumar DK - https://www.drupal.org/u/heykarthikwithu
The name reported in the user profile is Karthik Kumar D K, not Karthik Kumar DK.
-core: 8.x +core_version_requirement: ^8
core_version_requirement
is not recognized by any Drupal version that is lower than 8.8. Eithercore
is left, or the core requirement is changed.- - invite + - drupal:invite
The Invite โ module is not a Drupal core module.
/** - * Class RegisterSubscriber. + * Defines a controller to registration Subscriber. */ class RegisterSubscriber implements EventSubscriberInterface {
That class is not a controller.
+ /** + * The \Drupal\registration_invite\EventSubscriber\RegisterSubscriber object. + * + * @param \Drupal\Core\Config\ConfigFactory $configFactory + * The config factory service. + * @param \Drupal\Core\Session\AccountProxyInterface $current_user + * The account proxy service. + */ + public function __construct(ConfigFactory $configFactory, AccountProxyInterface $current_user) {
The short description for a class constructor starts with Constructs a new.
- Status changed to Needs review
over 1 year ago 10:27am 8 May 2023 - ๐ฎ๐ณIndia mrinalini9 New Delhi
Updated patch #28 by addressing #30, please review it.
Thanks!
- ๐ฎ๐ณIndia arpitk
Hi @mrinalini9 I reviewed the patch #31 Applies cleanly on version
8.x-1.x-dev. The changes from #30 also has been taken care. Only the error produced but phpcs as follows:C:\xampp\htdocs\d8\web\modules\contrib\registration_invite> phpcs --standard=Drupal,DrupalPractice --extensions=php,module,inc,install,test,profile,theme,css,info,txt,md,yml .
FILE: ...modules\contrib\registration_invite\registration_invite.info.yml
----------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
----------------------------------------------------------------------
1 | WARNING | "core_version_requirement" property is missing in the
| | info.yml file
----------------------------------------------------------------------As mention in #30 regarding core_version_requirement. This remains.
Thanks!
- Status changed to Needs work
over 1 year ago 1:25pm 7 June 2023 - ๐ฎ๐นItaly apaderno Brescia, ๐ฎ๐น
core: 8.x
I should have been clear about that. The core requirements should one of the following.
core: 8.x core_version_requirement: ^8 || ^9
core_version_requirement: ^8.8 || ^9
The latter is not equivalent to the first, as the first includes Drupal versions like Drupal 8.1, which are excluded from the latter.
+ /** + * Constructs a new RegisterSubscriber object. + *
The class namespace is missing.
- Assigned to sourabhjain
- Issue was unassigned.
- Status changed to Needs review
over 1 year ago 1:59pm 8 June 2023 - ๐ฎ๐ณIndia sourabhjain
Fixed the issue mentioned in #33. Please review.
- last update
over 1 year ago Composer require failure Hi, patch #35 failed to apply.
$ git apply 2958038-35.patch -v Checking patch README.txt... error: while searching for: * The administrative user interface can be found at http://yoursite.com/admin/config/people/accounts * Under Registration and cancellation section, select "New user registration by invitation only." MAINTAINERS error: patch failed: README.txt:38 error: README.txt: patch does not apply Checking patch registration_invite.info.yml... error: while searching for: name: 'Registration Invite' type: module description: 'User registration by invitation only' core: 8.x package: 'Custom' dependencies: - invite error: patch failed: registration_invite.info.yml:1 error: registration_invite.info.yml: patch does not apply Checking patch registration_invite.routing.yml... error: while searching for: defaults: _controller: '\Drupal\registration_invite\Controller\RegisterRedirect::content' requirements: _permission: 'access content' error: patch failed: registration_invite.routing.yml:3 error: registration_invite.routing.yml: patch does not apply Checking patch registration_invite.services.yml... error: while searching for: services: registration_invite.default: class: Drupal\registration_invite\EventSubscriber\RegisterSubscriber arguments: [] tags: - { name: event_subscriber } error: patch failed: registration_invite.services.yml:1 error: registration_invite.services.yml: patch does not apply Checking patch src/Controller/RegisterRedirect.php... error: while searching for: use Drupal\Core\Controller\ControllerBase; /** * Class RegisterRedirect. */ class RegisterRedirect extends ControllerBase { error: patch failed: src/Controller/RegisterRedirect.php:5 error: src/Controller/RegisterRedirect.php: patch does not apply Checking patch src/EventSubscriber/RegisterSubscriber.php... error: while searching for: namespace Drupal\registration_invite\EventSubscriber; use Symfony\Component\EventDispatcher\EventSubscriberInterface; use Symfony\Component\EventDispatcher\Event; use Symfony\Component\HttpFoundation\RedirectResponse; use Symfony\Component\HttpKernel\KernelEvents; use Symfony\Component\HttpKernel\Event\GetResponseEvent; /** * Class RegisterSubscriber. */ class RegisterSubscriber implements EventSubscriberInterface { /** * {@inheritdoc} */ public function checkIfReferred(GetResponseEvent $event) { $request = $event->getRequest(); $config = \Drupal::config('user.settings'); if ($request->attributes->get('_route') == 'user.register' && $config->get('register') === 'invie_only') { $access = TRUE; if (empty($_SESSION['invite_code']) && !(\Drupal::currentUser()->hasPermission('administer uses'))) { $access = FALSE; } if (!$access) { error: patch failed: src/EventSubscriber/RegisterSubscriber.php:3 error: src/EventSubscriber/RegisterSubscriber.php: patch does not apply Checking patch templates/registration-invite.html.twig... error: while searching for: <!-- Add you custom twig html here --> error: patch failed: templates/registration-invite.html.twig:1 error: templates/registration-invite.html.twig: patch does not apply
So changing the status to Needs work.
Thankyou.- last update
10 months ago 2 fail - First commit to issue fork.
- last update
9 months ago 2 fail