I did a quick review of the code after 8.x-1.0-beta1 release. Thanks for maintaining the module and contributing to D8.

autologout.module

I highly encourage to convert the API functions in to an autologout service. It allows others to override the core of the module without hacking it. And in general play better with D8 architecture.

autologout.api.php

Example code still contains the D7 arg() function.

AutologoutController.php

Method names do not need to be prefixed with the module name. The class covers the namespace.

autologout.settings.yml

Variable names do need to be prefixed with the module name. The config file covers the name space.

Comments

Sutharsan created an issue. See original summary.

AjitS’s picture

Assigned: Unassigned » AjitS

Thanks a lot for your review and feedback. Will look into the points and work on them individually.
Assigning to myself :)

abhishek-anand’s picture

A pull request had been created to address the above changes.

https://github.com/ajitdev/autologout/pull/18

abhishek-anand’s picture

Status: Active » Needs review

  • abhishek-anand committed 5cb1539 on 8.x-1.x
    Issue #2629536, by abhishek-anand: AutologoutController.php Method names...
  • abhishek-anand committed 6af2d4a on 8.x-1.x
    Issue #2629536, by abhishek-anand: Fix Example code still contains the...
  • abhishek-anand committed bc515b8 on 8.x-1.x
    Issue #2629536, by abhishek-anand: autologout.settings.yml Variable...
naveenvalecha’s picture

Status: Needs review » Fixed

The suggested changes has been pushed to the dev branch

Status: Fixed » Closed (fixed)

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

AjitS’s picture

AjitS’s picture