The crss feed is not shown if we're on a front page and current path is 'frontpage'.

    case COMMENTRSS_SITE_FRONT_AND_NODE_PAGE:
      if (!drupal_is_front_page() || (current_path() != 'node')) {
        // Only break if not front page and not node page.
        break;
      }

The comment and the code contradict each other. It seems the comment is correct.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

scito’s picture

Or may be the order of COMMENTRSS_SITE_FRONT_AND_NODE_PAGE and COMMENTRSS_SITE_FRONT_PAGE should be exchanged.

The possible cases should be examined.

rodrigoeg’s picture

Issue summary: View changes
Status: Needs review » Needs work

The patch is working fine. Only the patch file name is not following the standards mentioned on this page https://www.drupal.org/patch/submit

I think this approach is fine (I am not the maintainer of the module), and it also follows the implementation mentioned on the comments "Only break if not front page AND not node page."

davic’s picture

Fixed name of the patch submitted by scito according to standards.

davic’s picture

Status: Needs work » Needs review

Fixed the name of the patch submitted by scito , ready for review.

The last submitted patch, commentrss_fix_condition.patch, failed testing.

The last submitted patch, commentrss_fix_condition.patch, failed testing.

The last submitted patch, commentrss_fix_condition.patch, failed testing.

The last submitted patch, commentrss_fix_condition.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 3: commentrss-feed_front-1704724-3-D7.patch, failed testing.

The last submitted patch, 3: commentrss-feed_front-1704724-3-D7.patch, failed testing.

The last submitted patch, 3: commentrss-feed_front-1704724-3-D7.patch, failed testing.

The last submitted patch, 3: commentrss-feed_front-1704724-3-D7.patch, failed testing.

davic’s picture

Remade the patch made by scito due to test fail on both previous patches.

davic’s picture

Status: Needs work » Needs review

Reworked the patch scito made due to test fail.

Status: Needs review » Needs work

The last submitted patch, 13: commentrss-comment-expose-front-1704724-13-D7.patch, failed testing.

The last submitted patch, 13: commentrss-comment-expose-front-1704724-13-D7.patch, failed testing.

The last submitted patch, 13: commentrss-comment-expose-front-1704724-13-D7.patch, failed testing.

The last submitted patch, 13: commentrss-comment-expose-front-1704724-13-D7.patch, failed testing.

davic’s picture

Test CI error, but patch is working fine. May need patch test reroll.

larruda’s picture

Status: Needs work » Needs review

This module does not implement its own tests so I have disabled Automated Testing for it on DrupalCI.
Changing it back to Needs Review. Someone please go ahead and review it.

Thanks!

Diego_Mow’s picture

Tested and working fine. RTBC

  • larruda committed 0078bcd on 7.x-2.x authored by davic
    Issue #1704724 by davic, scito, Diego_Mow, rodrigoeg: Feed exposition...
larruda’s picture

Status: Needs review » Fixed

Thank you guys!

Diego_Mow’s picture

Status: Fixed » Reviewed & tested by the community
larruda’s picture

@Diego_Mow, I have just committed this to the dev branch and now it's fixed. Any reason to change it back to RTBC?

larruda’s picture

Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.