Skip to content

Conversation

@chetansatasiya
Copy link

Feature #126

@philipjohn
Copy link
Contributor

Some feedback on this in #126

@philipjohn philipjohn requested a review from tomjn June 17, 2017 15:28
@GaryJones GaryJones changed the base branch from master to develop February 11, 2024 12:25
$post_source = get_post_meta( $post->ID, 'syn_source_url', true );
if ( isset( $post_source ) && '' !== $post_source ) {
echo '<div class="syn-post-source">';
echo esc_html( sprintf( 'Source: %s', parse_url( $post_source, PHP_URL_HOST ) ), 'push-syndication' );
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks okay but 2 points:

  • There's no internationalisation here for Source:
  • If the post source is not empty and not a valid URL then it will return false or null, but sprintf is expecting a string, which could lead to some oddness.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only took you 7 years, 8 months and 22 days Tom 😂

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I needed to think long and hard about this one

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants