[D8] Current Page Crumb

Fixed the git clone URL in the issue summary for non-maintainer users.

We are currently quite busy with all the project applications and we prefer projects with a review bonus. Please help reviewing and put yourself on the high priority list, then 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.

Looks good and nice utility module. Thanks for contributing it.

Functionally, I feel the assumption to excluding admin path might be avoided by introducing it as a config item and keeping disabled by default.

Thanks @Greg Boggs. I didn't know about easy_breadcrumb. I have installed this module and it works as explained here. Here is some more review comments:

1. Ideally we should try to avoid getting service inside class by using static methods(It's more intended for procedural part of code). The services should be injected to the class. However, we already have title_resolver in parent class. So we can just replace \Drupal::service('title_resolver') with $this->titleResolver
2. Same as #1, but we don't have router.admin_context inside this class. However, all isAdminRoute() does is check for $route->getOption('_admin_route'). as we have $route, we can use it.

3.       - { name: breadcrumb_builder, priority: 1 }
   

   Not sure making priority as 1 is a good idea. Most of the core non-system modules start with 1000. I guess, if we get another breadcrumb builder that should get executed between this and system's, then may be we can override the priority of current_page_crumb.breadcrumb in /sites/default/services.yml?

4. It would nice to have some tests :)

P.S I didn't run the coder/code sniffer (I don't know if they are mandatory for contrib). but would be "nice to have" them fixed as well :)

What priority would you recommend in this case, 1000?

It doesn't look like a Symfony thingy. Something we introduced in Drupal. So I guess anything between 0 to 1000 should be fine.

I reviewed the code and it is simple and readable. The amount of comments are appropriate for the amount of code.

It appears you are working in the "master" branch in git. You should really be working in a version specific branch. The most direct documentation on this is Moving from a master branch to a version branch. For additional resources please see the documentation about release naming conventions and creating a branch in git.

PAReview still says there's an issue with the branch. I think what it is, is the "default branch" has not been specified. You're committing to 8.x-1.x, but still have to tell git that it is the default. Look at the "Moving from a master branch to a version branch" to set the default.

Thanks for your contribution, Greg!

I updated your account so you can 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 stay 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.

yay! thanks @DamienMcKenna.