Month Year Range, Provide the form widget to fill in the month and year range or year range only . This is just a change to the display form, so that the user doesn't need to enter too much content.
This module not stored time data. The storage of time is provided by the core.
Comments
Comment #2
shashank5563 commentedThank you for applying! Reviewers will review the project files, describing what needs to be changed.
Please read Review process for security advisory coverage: What to expect for more details and Security advisory coverage application checklist to understand what reviewers look for. Tips for ensuring a smooth review gives some hints for a smoother review.
To reviewers: Please read How to review security advisory coverage applications, What to cover in an application review, and Drupal.org security advisory coverage application workflow.
While this application is open, only the user who opened the application can make commits to the project used for the application.
Reviewers only describe what needs to be changed; they don't provide patches to fix what reported in a review.
Comment #3
vinaymahale commentedRan PHPCS tests. No PHPCS-related issues were found. Let's wait for other reviewers
Comment #4
shashank5563 commentedComment #5
shashank5563 commented@nikral, There are three branches. It is difficult review to the reviewer. Please remove the unused branch. So, we can review the your module.
Comment #6
shashank5563 commentedPlease use the branch instead of the tag in the title.
Like: [9.0.5] Month Year Range into [1.x] Month Year Range
Comment #7
nikral commentedThanks @shashank5563
Comment #8
avpadernoI do not have time for a complete review, but there is something that needs to be fixed.
Projects hosted on drupal.org are licensed under GPLv2+, the same license used from Drupal core. If you are licensing a project under a different license, it cannot he hosted on drupal.org. More details are given in Drupal Git Contributor Agreement & Repository Usage Policy.
For code licensed under GPLv3, see See I want to release my work under GPL version 3 or under GPL version 2-only. Can I do so and host it on Drupal.org?
Comment #9
nikral commentedthanks @apaderno,
I just deleted the license and unnecessary files.
Comment #10
shashank5563 commented@nikral , I have reviewed the changes, and they look fine to me.
Let’s wait for other reviewers to take a look and if everything goes fine, you will get the role.
Comment #11
sgourebi commented@nikral
In the file src/Plugin/Field/FieldWidget/MonthYearRangeWidget.php
You don't have to implement ContainerFactoryPluginInterface because it is almost implemented in the web/core/lib/Drupal/Core/Field/WidgetBase.php
In the file month_year_range.info.yml
Drupal 8 is not yet supported, so I guess you have to set the core version like this
core_version_requirement: ^9 || ^10I you want to keep compatibility with D8, you have to set the
core_version_requirement: ^8.8 || ^9 || ^10Or
add the core key.
core: 8.xto support version before D8.8 because D8.7.7 doesn't support core_version_requirement.core_version_requirement: ^8 || ^9 || ^10I ran PHPCS tests and no related coding standard issues were found.
Except what I described, it is good for me.
Comment #12
nikral commentedThanks @shashank5563 and @sgourebi for the review,
@sgourebi, I removed the implementation of ContainerFactoryPluginInterface in src/Plugin/Field/FieldWidget/MonthYearRangeWidget.php
For the version, I think that for the moment we can leave it like this
core_version_requirement: ^8 || ^9 || ^10Comment #13
avpadernoTo make clear how to declare the required Drupal core versions:
core_version_requirement: ^8 || ^9 || ^10andcore_version_requirement: ^8.8 || ^9 || ^10have the same effect on Drupal versions before Drupal 8.7.7, as in both the cases the Drupal site would throw an error, since those Drupal versions do not recognize thecore_version_requirementkey. For them, it is like no core requirement has been defined.The only difference is that, using
core_version_requirement: ^8 || ^9 || ^10, the users will read Works with Drupal: ^8 || ^9 || ^10 on the project page and think they can install it on any Drupal 8 version, when it is not so.core: 8.xcannot be used with any value ofcore_version_requirementthat is not^8 || ^9. Usingcore: 8.xtogethercore_version_requirement: ^8 || ^9 || ^10would throw an error on Drupal versions after Drupal 8.7.7.Comment #14
sgourebi commentedIt is ok for me now.
Comment #15
vinaymahale commentedI am changing priority as per Issue priorities.
Comment #16
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 Slack #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 reviewers.
Comment #17
avpaderno