Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Some code needs cleanup.
I found, i.e., that the function referral_validate_multiplier() is written like:
function referral_validate_multiplier($form) {
if ($form['#value'] >= 0 && $form['#value'] <= 100) {
return;
}
else {
form_set_error('referral_points_multiplier', t('Percentage multiplier must be between 0 and 100'));
}
}
where it should be write like:
function referral_validate_multiplier($form) {
if ($form['#value'] < 0 || $form['#value'] > 100) {
form_set_error('referral_points_multiplier', t('Percentage multiplier must be between 0 and 100'));
}
}
| Comment | File | Size | Author |
|---|---|---|---|
| #8 | userpoints_retroactive_module.patch | 1.52 KB | avpaderno |
| #7 | userpoints_reserve_module.patch | 3.16 KB | avpaderno |
| #6 | userpoints_nodelimit_module.patch | 851 bytes | avpaderno |
| #5 | userpoints_no_negative_module.patch | 968 bytes | avpaderno |
| #4 | userpoints_expire_module.patch | 1012 bytes | avpaderno |
Comments
Comment #1
kbahey commentedPlease read http://drupal.org/patch carefully and follow the instructions there on creating proper patches.
This will make reviewing them easier and faster.
Comment #2
kbahey commentedComment #3
avpadernoComment #4
avpadernouserpoints_expirestill uses the fieldevent, which has been renamedoperation. Plus, the Drupal coding standards suggests to use two spaces indentation, without tabs.Comment #5
avpadernoComment #6
avpadernouserpoints_nodelimit_form_alter()completely removes all the fields of the node editing form, which is a "dangerous" operation.It would be sufficient to remove the form buttons, so the form could not be submitted.
Comment #7
avpadernouserpoints_reserve.moduledoesn't output well-formed XHTML.Comment #8
avpadernoIn
userpoints_retroactive.modulethe functionuserpoints_retroactive_page()doesn't even works as it uses a not initialized variable:Comment #9
avpadernouserpoints_top_contributors.module, anduserpoints_votingapi.moduledon't usehook_userpoints()to add more settings in theuserpoints.modulesettings form.Comment #10
kbahey commentedKiam
What you are doing is very confusing.
Please open one issue for each separate problem that has a different patch.
I am closing this issue.
Comment #11
avpadernoShall I open more than one issue for what is just a general cleanup task?
Comment #12
kbahey commentedKiam
If it touches several modules, and each module has a separate patch, then it needs to be in separate issues so they are committed and tracked separately.
If you have one issue with 5 patches for 5 modules, then we can't track things at all.
They cannot be even reviewed separately.
I told you what needs to be done. If you want it committed, then do it the way everyone does it.
Comment #13
jredding commentedJust for reference so that you understand the reasoning behind this (other than tracking the patch review/commit status)
Userpoints Contributed modules is a collection of individually submitted modules by many different authors. Because they are small in nature they are offered as a collection instead of individually downloadable modules.
What this means though is that there is not a single maintainer for the collection of modules but almost one maintainer per module within the collection (that is if there is a maintainer at all, which often is not the case). It is much better to provide patches, tasks, issues, etc. against the individual pieces within the collection so that they can be better categorized.
The hope is that once an individual module becomes popular and an individual maintainers comes forward they will be branched out into their own individual modules.
for example
http://drupal.org/project/userpoints_ubercart
Comment #14
avpadernoComment #15
avpadernoThe issue has been spitted in multiple issues, like suggested me to do.
I consider this issue closed.
-- Kiam@AVPnet