This module is helping with the clustering of points on the server-side. These clusters are then displayed on a map (through other modules)
This has much better performance than client-side when there are 10,000+ points to cluster
There is the possibility to further improve performance by adding new algorithm (as plugins)
It's a port of the D7 module with various fixes & improvements, plus trying to leverage D8 concepts as much as possible.
Project link
https://www.drupal.org/project/geocluster
Git instructions
git clone --branch 8.x-1.x https://git.drupalcode.org/project/geocluster/
PAReview Checklist
https://pareview.sh/pareview/https-git.drupal.org-project-geocluster.git...
Comment | File | Size | Author |
---|---|---|---|
#8 | Screenshot 2019-12-19 at 15.53.34.png | 329 KB | vuil |
Comments
Comment #2
vdsh CreditAttribution: vdsh as a volunteer commentedComment #3
vdsh CreditAttribution: vdsh as a volunteer commentedComment #4
vuilThank you for the D8 project contribution!
I saw that maintainer @das-peter of the project (module) has rights to produce Security Coverage and stable version. Am I right?
Comment #5
vdsh CreditAttribution: vdsh as a volunteer commentedHi Vuil,
And thanks for your comment. Yes, I also think das-peter has the rights to produce Security Coverage versions.
However, as he told me, he is busy with other projects and didn't have much time to work on this for now (last commit quite some time ago)
And I am also co-maintainer for another related module https://www.drupal.org/project/leaflet_geojson which is also not really maintained. So I think it's easier if I can be vetted to get Security Coverage rights and able to do releases covered by Drupal’s security advisory policy.
Comment #6
apadernoAs long as the user who applies is the only user who commits code in the branch used for the application, we don't check if the other maintainers have the role to opt into security coverage, especially in the case the project owner ha already the role to opt into security coverage.
Comment #7
vuilOK, Thank you for the contribution!
1. Please use the better construction as
instead of
in
geocluster.install
file and inDrupal\geocluster\GeoclusterConfig
class.2. Please replace deprecated
drupal_set_message()
with\Drupal::messenger()->addMessage()
inDrupal\geocluster\GeoclusterConfig
.3. Update your
t()
methods with$this->t()
and adduse StringTranslationTrait;
statement under class declaration:4 Please remove one of the lines
in
Drupal\geocluster\Utility\GeoclusterHelper
5. Please fix the
__construct()
method issue inDrupal\geocluster\Plugin\GeoclusterAlgorithmBase
(attached screenshot).6. There are missing
@return
tags in code annotations, please fix all of them.Comment #8
vuilI added the screenshot of #7 5.) issue.
Comment #9
vdsh CreditAttribution: vdsh as a volunteer commentedThanks vuil for this detailed analysis. I have corrected all the things you mentionned (and slightly cleaned further)
Comment #10
klausiThanks for your contribution!
Otherwise looks good to me, did not see security issues.
Comment #11
apadernoThank 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 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.