Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IvooxBridge] new bridge for podcast plataform #597

Merged
merged 4 commits into from
Mar 22, 2019

Conversation

xurxof
Copy link
Contributor

@xurxof xurxof commented Nov 11, 2017

No description provided.

Copy link
Member

@teromene teromene left a comment

Choose a reason for hiding this comment

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

Just few corrections before I can merge. Thanks !

private function ivGetSimpleHTMLDOM($url)
{
return getSimpleHTMLDOM(
$url,
Copy link
Member

Choose a reason for hiding this comment

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

No whitespace at end of line.
Same line 25, 26, 38, 29, 30, 31, 32, 33, 34, 60, 63, 67, 96, 109

}
}

private function ivBridgeAddItem($episode_link, $podcast_name, $episode_title, $author_name, $episode_description, $publication_date, $episode_duration)
Copy link
Member

Choose a reason for hiding this comment

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

Line too long, you can use \ followed by line return

$item['author'] = $author_name;
$item['timestamp'] = $publication_date;
$item['uri'] = $episode_link;
$item['content'] = '<a href="' . $episode_link . '">' . $podcast_name . ': ' . $episode_title . '</a><br />Duration: ' . $episode_duration . '"<br />Description:<br />' . $episode_description;
Copy link
Member

Choose a reason for hiding this comment

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

Line too long as well

if ($linkcount == 0) {
$episode_link = $link->href;
$episode_title = $link->title;
}
Copy link
Member

Choose a reason for hiding this comment

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

Space after closing bracket, same line 78, 118, 131

$episode_duration = $flipper->find('p.time', 0)->innertext;
$publication_date = $flipper->find('li.date', 0)->getAttribute('title');

// alternative date_parse_from_format or DateTime::createFromFormat('G:i - d \d\e M \d\e Y', $publication); // TODO: month name translations, due funciton doesn't support locale
Copy link
Member

Choose a reason for hiding this comment

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

Line too long, you can use a multi-line comment

$a = strptime($publication_date, '%H:%M - %d de %b. de %Y'); // obsolete function, uses c libraries
$publication_date = mktime(0, 0, 0, $a['tm_mon'] + 1, $a['tm_mday'], $a['tm_year'] + 1900);

$this->ivBridgeAddItem($episode_link, $podcast_name, $episode_title, $author_name, $episode_description, $publication_date, $episode_duration);
Copy link
Member

Choose a reason for hiding this comment

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

Line too long as well

@teromene teromene dismissed their stale review April 15, 2018 10:44

New review

Copy link
Member

@teromene teromene left a comment

Choose a reason for hiding this comment

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

Moreover, not an issue on your side but can you change the "=" on lines 135 and 136 to their ascii codes (3d), as it is messing with phpcs

@@ -42,29 +42,32 @@ private function printIfDebug($text)
}
}

private function ivBridgeAddItem($episode_link, $podcast_name, $episode_title, $author_name, $episode_description, $publication_date, $episode_duration)
private function ivBridgeAddItem($episode_link, $podcast_name, $episode_title, $author_name,
$episode_description, $publication_date, $episode_duration)
Copy link
Member

Choose a reason for hiding this comment

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

Please use tabs, no spaces

{
$item = array();
$item['title'] = $podcast_name . ':' . $episode_title;
$item['author'] = $author_name;
$item['timestamp'] = $publication_date;
$item['uri'] = $episode_link;
$item['content'] = '<a href="' . $episode_link . '">' . $podcast_name . ': ' . $episode_title . '</a><br />Duration: ' . $episode_duration . '"<br />Description:<br />' . $episode_description;
$item['content'] = '<a href="' . $episode_link . '">' . $podcast_name . ': ' . $episode_title . '</a>'
. '<br />Duration: ' . $episode_duration . '"'
Copy link
Member

@teromene teromene Apr 15, 2018

Choose a reason for hiding this comment

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

No need to concatenate '</a>' and '<br />Duration: '.

@logmanoriginal logmanoriginal dismissed teromene’s stale review March 22, 2019 20:33

I've applied the necessary fixes and made sure it works as intended

@logmanoriginal logmanoriginal merged commit 18d5ef1 into RSS-Bridge:master Mar 22, 2019
infominer33 pushed a commit to web-work-tools/rss-bridge that referenced this pull request Apr 17, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants