backdrop-contrib / honeypot

Backdrop port of Drupal module. Uses both the honeypot and timestamp methods of deterring spam bots from completing forms on your site.
GNU General Public License v2.0
3 stars 1 forks source link

I would like to become a maintainer for honeypot #17

Open jenlampton opened 7 years ago

jenlampton commented 7 years ago

Hi @herbdool! I have a few changes I would like to make to honeypot module, and would like your permission to become a maintainer for the project.

In particular I would like to:

Or please let me know if you would prefer pull requests for the first 2 issues instead :)

herbdool commented 7 years ago

@jenlampton I'm usually on top of PRs so that's ok too. I don't mind adding you as a maintainer but prefer we have PRs reviewed before merging.

I'm not sure about caching. Caching is disabled because it messes up the timestamp for the time limit. Unless the time limit is disabled by default (I'll double check) the caching can't be enabled.

I found this issue for a workaround for page caching https://www.drupal.org/node/2820400 but there might be issues with it.

I'm not sure about changing version numbers either. It's using semver where the Drupal project isn't plus there isn't much development on the Drupal 7 version so I can't see it being necessary. Unless I'm missing something.

herbdool commented 7 years ago

I don't think we can attempt the honeypot ajax solution in the link above until https://www.drupal.org/node/2819375 is accepted into Drupal 7.60 or about and merged into Backdrop.

jenlampton commented 7 years ago

@jenlampton I'm usually on top of PRs so that's ok too. I don't mind adding you as a maintainer but prefer we have PRs reviewed before merging.

In that case you should be listed as a maintainer in the readme. I didn't want to submit a bunch of Issues and PRs knowing there would be no one to merge them when done, without also offering to help with that part. If you are willing to maintain we should add you to the readme so it won't prevent others in my position from doing the same :) See https://github.com/backdrop-contrib/honeypot/issues/18

I'm not sure about caching. Caching is disabled because it messes up the timestamp for the time limit. Unless the time limit is disabled by default (I'll double check) the caching can't be enabled.

Yes, that's what I meant. Let's discuss here: https://github.com/backdrop-contrib/honeypot/issues/20

I'm not sure about changing version numbers either. It's using semver where the Drupal project isn't plus there isn't much development on the Drupal 7 version so I can't see it being necessary. Unless I'm missing something.

Here is Backdrop's recommended version numbering from api.backdropcms.org:

When tagging your Backdrop module for release, please use the same version number as the Drupal module being ported. For example, if you are porting the 7.x-4.15 version of webform module, that should become the 1.x-4.15.0 version of the Backdrop module. Any bug fixes would increment the last number to 4.15.1 or 4.15.2.

The benefit here is that when I find a problem in the backdrop module, with a solution in the Drupal module, I can tell if the backdrop version is supposed to contain that fix if the version numbers match. If not, I need to do a lot of Git magic to find a commit. It's still possible to find it, of course, but you'll be making life easier for maintainers (and possible contributors!) if the numbers line up cleanly.

herbdool commented 7 years ago

@jenlampton I agree in principle with using the Drupal version numbers. I'm guessing that recommendation was added recently? I don't recall it being there before so I've seen--and used--different approaches for releases as I've learned.

I'm just wondering what disruption there will be if there's a jump in the version number for already existing releases. Perhaps the next release could be the new numbering with a note why.

Aside: even webform isn't following that standard and it would be a huge jump if we changed it now. It's on 1.x-1.6.x and changing it to 1.x-4.15 would probably be confusing and mean that we'd also have to update the hook_update number. In that case I'd probably suggest making it 1.x-1.15 and note (as I've already done in previous releases) that it matches 7.x-4.15.

laryn commented 7 years ago

@herbdool @jenlampton Yes, is that a new recommendation regarding version numbers? I asked about this on Gitter relatively recently and was left with the impression that there was no official recommendation on it. I see the pro's to doing it that way, but the con that didn't have a solid solution was what to do when/if functionality begins to diverge from the Drupal module and we're still tied to version numbers that track the D7 module. One solution which didn't involve syncing version numbers was to include in the release description which version of the Drupal module was encompassed in that release.

jlfranklin commented 7 years ago

I see the pro's to doing it that way, but the con that didn't have a solid solution was what to do when/if functionality begins to diverge from the Drupal module and we're still tied to version numbers that track the D7 module.

If you're tracking a project, track its version number, too. When you start to diverge, but you are still tracking the original, add "+bd1.0" to the end, or something like that. Debian uses this when adding patches to existing upstream projects.

For example, the current version of the Linux kernel is 4.9.30-2+deb9u3. Tracking the version number lets users track not just bugs, but also functionality. If I need kernel modules that were released in 4.9.x, then I know this is the right one.

When the code diverges so much that you're no longer tracking it, fork it proper, and give the module a new name to distinguish it from the original.

jenlampton commented 7 years ago

I'm guessing that recommendation was added recently?

Yes, I think it was added after the Gitter conversation with @laryn.

I'm just wondering what disruption there will be if there's a jump in the version number for already existing releases.

A jump up in number shouldn't present any problem at all. Even in the case of webform, as big a jump as 1.x-1.6.x to 1.x-4.15.x wouldn't need any changes in code. (A jump down should be avoided at all costs, however).

One solution which didn't involve syncing version numbers was to include in the release description which version of the Drupal module was encompassed in that release.

Yep, and that's also in the docs :)

If the Backdrop and Drupal version numbers do not match for any reason, the Backdrop release description should contain the Drupal version that is comparable.

When the code diverges so much that you're no longer tracking it, fork it proper, and give the module a new name to distinguish it from the original.

This is not a good idea. We'll get tons of similar/duplicate projects with no workable upgrade path (and open source projects like Drupal have enough of that problem already). I think if we ever stop tracking Drupal updates it would be best to simply say so, and say upgrades after a certain version are not guaranteed. Fortunately with Drupal 7's end of life in sight, we won't need to track those changes forever.

Anyway @herbdool if you are the maintainer for this project you are welcome to change/keep the version numbering however works best for you. :) I was only recommending a change because I was offering to maintain it, and since I had some difficulty with not knowing which Drupal version had been forked, I had that solution in mind.