[D8] Time Field

Hello Amin,

Please run automated review and check the errors.

Here is the report. https://pareview.sh/pareview/https-git.drupal.org-project-time_field.git

Thanks

Thank you for your contribution!

You need to provide the instructions to clone the project repository, and create a branch for Drupal 8. The Master branch should not be used to commit code.

Please don't set the issue to Fixed, or you will never get the permission to opt-in to security coverage.

@amin,

Your README.md does not follow best practices (headings need to be uppercase). See https://www.drupal.org/node/2181737 .
- The REQUIREMENTS section is missing.
- The CONFIGURATION section is missing.

Please use t() function wherever you add static string/message.

Please fix it.

Thanks.

We just need to wait for other users to review the code. It's their task, to set the issue as Reviewed & tested by the community.

As a reminder to the reviewers: This application is done to see what the OP understands about writing secure Drupal code. It should not become a find all the bugs in the project session, even though we are also interested in the OP's ability to understand bug reports and correct the code basing on the report.

Review of the 8.x-1.x branch (commit 58d429b):

• PAReview still has some issues: https://pareview.sh/pareview/https-git.drupal.org-project-time_field.git.
  • I would recommend you to fix them simply by moving Assert library out of your module (use composer dependency with instructions inside README instead of including someone's else library).
  • But to be honest, I don't understand why library is needed here - you can simply implement the only used range() function yourself (it can be simpler version) and remove that library at all.
• Git repository and README is now nice and clean.
• Description and package inside time_field.info.yml need to be changed - please provide some short description and change package to "Field types".
• I would remove return type declaration from "baseDateTime(): \DateTime" (it's enough to have it inside annotation).
• Inside TimeType.php "isEmpty()" function, returning "empty($value)" should work the same way. Similarly inside TimeRangeType.php. Annotations for these 2 classes are missing "@Translation" for "category".
• Validations, field widgets and formatters are nicely done.
• In general, nice code inside your module, please fix mainly the library and info yml, thanks.

Review of branch 8.x-1.x
Automated review didn't show any errors.

Manual review:
Code looks clean and working.

1. I just noticed very minor issue: could you add 2 lines before headings in readme.md
(Two lines prior to headings (except the first one)).

2. I'm not sure about coding standard of 'native' PHP classes, like Exception or DateTime. What is preferred, inline classes (with backslash before class name: \DateTime) or use statement on top (use DateTime; )? Anyway it works both ways, so this is not an issue.

Please fix 1st issue and I update issue as reviewed and tested.

Hello,

Review of branch 8.x-1.x

Automated review looks ok, don't see any errors.

Manual review:
Code looks clean and working (tested on real project).

I see just few very minor issues:
1. 'templates' folder should be moved from 'src' to project root. It should be like that 'time_field/templates'.
2. @file notation with list of available variables should be added for `input--time.html.twig` template. Example:

{#
/**
 * @file
 * Theme override for an 'input' #type form element.
 *
 * Available variables:
 * - attributes: A list of HTML attributes for the input element.
 *
 * @see template_preprocess_input()
 */
#}

That's all, other points looks completely ok for me.

PAReview doesn't report any issue and I didn't find any security issue.

As side note, I guess that by Seconds passed through midnight you mean Seconds passed since midnight.

I will approve this application this evening.

Thank you for your contribution!
I am going to update your account so you can opt into security advisory coverage now.
These are some recommended readings to help with excellent maintainership:

• Dries' post on Responsible maintainers
• Best practices for creating and maintaining projects
• Maintaining a drupal.org project with Git
• Commit messages - providing history and credit
• Release naming conventions.
• Helping maintainers in the issue queues

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

The project needs to be edited; this application just gives you the permission to edit the Security advisory coverage field in https://www.drupal.org/node/2977064/edit (and in other projects you created, or you will create).