[D7] Avoid 404

interesting approach. I'm tempted to install it on my website just to see how long it takes to respond to a 404, on a site with *lots* of path aliases.

You need a README.txt file, especially if there is anything that needs to be done for installation other than visiting the modules page.

You need to delete the .svn directory from the git repository.

Dunno if it's possible with a sandbox, but your branch should be "7.x-1.x" rather than "master".

You need to run everything through Coder review if you haven't already.

Consider renaming your files/functions to "avoid404" instead of "avoid_404" (i.e. lose the underscore) This is a suggestion not a requirement. Even if someone created a module called "avoid", I doubt we'll ever have any hooks called "404" so probably there's no possibility of namespace clashing.

You need an avoid_404.install (or avoid404.install) file to delete your module-specific variables when your module gets uninstalled. See hook_uninstall().

Probably you should consider integrating with the Redirect module. (This is another suggestion.)

I'd be curious to know what "hoechste" means in English.

Yup; I can see you have not run Coder review, because you have trailing spaces.

The avoid_404_getRedirectAlias() function should be renamed to avoid_404_get_redirect_alias(). Drupal coding standards reserves the use of MixedUpperAndLowerCase for class and method names. Ordinary functions should use the underscore as a word delimiter.

Your files have DOS line endings. You need to convert them to Unix line endings, or else coder will spit out some really confusing error messages.

Remove the ; $Id$ and the version = 0.1 lines from your avoid_404.info file.

(attempting to actually install and test the module now...)

It would be nice (another suggestion) if there was a "select all" option under "Menus to be considered".

(trying to visit a nonexistent page...)

Yup; your module definitely conflicts with the Redirect module.

(Temporarily disabling the latter for testing.)

Okay, I have a page at http://pillars.net/lyrics/rejoice

I tried to visit http://pillars.net/lyrics/rejoiced

But I got a 404 "Page not found" instead.

So... needs work. Too bad, because I'd really like to test things out on a real website.

So it would appear that your module has other conflicts with my site, as well.

The applicant has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

The application has been closed. If you would like to reopen it, you are free to do so.
See http://drupal.org/node/894256#abandonedtwoweekscontact

There are some errors reported by automated review tools, did you already check them? See http://ventral.org/pareview/httpgitdrupalorgsandboxhardcoding1266394git

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and we will take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (they found some problems with the project) or "reviewed & tested by the community" (they found no major flaws).

I'm a robot and this is an automated message from Project Applications Scraper.

2 minor code change suggestions in avoid404.module:

• Line 123: There should be no trailing spaces
• Line 132: in most cases, replace the string function with the drupal_ equivalent string functions
  (return substr(preg_replace('/' . trim($language->prefix) . '/', "", $path, 1), 1);)
  • use http://api.drupal.org/api/drupal/includes%21unicode.inc/function/drupal_...

Tested D7.22
Looks good.
Code looks good.
The only thing I would add is in the README.txt because currently there is no text in that file.

When I installed the module, I was not able to access the configuration page even though I am in the administrator role. You should implement hook_permission() so the permissions for this module are assignable to users and roles.

Otherwise, the rest of the code looks good to me. Good job!

Looks good! As far as I can tell, all you need to do now is participate in the Review Bonus program. Good luck.

changed git url

I can suggest you can remove "[D7]" from module name at http://drupal.org/sandbox/hardcoding/1266394.
D7 prefix in title is neccessary only in issues queue for project applications

Just tested this and works as stated. Nice, i think this module can really enhance the user experience.

Maybe a small optimisation would be to try to put all functions not needed directly in a separate file which you include only when going to do avoid404 specific stuff. I think from hook_init you only need 'avoid404_page_exists' -> 'avoid404_build_path' -> 'avoid404_get_active_languages'.
Then in your true condition, include the extra functions to do avoid404_goto

How about a hook to let other modules add their own list of extra aliases. Just a thought.

regards,

Have you looked into Search 404? I think the functionality of your module can be replicated by it, since it has options to show the first search result instead of the results page when a 404 is reached. I haven't really tested yours, so I might be wrong.

You should ensure your project is not a duplication as suggested in the project application checklist, and please if it is not a duplication add a comment in your project page explaining the differences between both modules.

manual review:

1. avoid404_get_selected_menu_names(): you could use drupal_map_assoc() instead of the loop?
2. avoid404_init(): hook_init() is bad because it runs on every single page request. I would rather suggest that you create your own hook_menu() entry and set the site_404 variable to that path. That way your callback will only run if an actual 404 was encountered. And you can move the original path that was in site_404 to another variable and invoke that page if there is not enough similarity with your matching attempts.
3. avoid404_admin_settings_submit(): since you are using system_settings_form() in the form builder anyway that function is not needed and can be removed.
4. avoid404_page_exists(): please document the return value with @return. Shouldn't you return booleans, i.e. TRUE instead fo "1"?

But that are not critical application blockers, so I guess this is RTBC. Removing review bonus tag, you can add it again if you have done another 3 reviews of other projects.

Assigning to Dave Reid as he might have time to take a final look at this.

no objections for more than a week, so ...

Thanks for your contribution, hardcoding!

I updated your account to let you promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here 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 lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and get involved!

Thanks, 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.

Thanks to the dedicated reviewer(s) as well.