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

W3Schools Plugin #88

Merged
merged 3 commits into from Oct 6, 2016
Merged

W3Schools Plugin #88

merged 3 commits into from Oct 6, 2016

Conversation

Nessworthy
Copy link
Contributor

PR For Issue #80.
Would like some initial eyes on this first, tests are passing but I aim to add more for coverage tomorrow.

@Nessworthy
Copy link
Contributor Author

Main concerns are copy and resource links. Specifically the use of w3fools for the main link, and the lines for MySQL and XML. There's also currently no support for specific W3 articles past the initial category.

Copy link
Member

@DaveRandom DaveRandom left a comment

Choose a reason for hiding this comment

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

Use HTTPS whenever possible please :-)


if(
!preg_match_all(
'/' . self::BAD_HOST_PATTERN . '\/(' . implode('|', $categoryNames) . ')\//i',
Copy link
Member

Choose a reason for hiding this comment

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

Since you need to match literal slashes, why not use a different delimiter?

e.g. '#' . self::BAD_HOST_PATTERN . '/(' . implode('|', $categoryNames) . ')/#i'

'css' => '[Check the Mozilla Developer Network CSS documentation](https://developer.mozilla.org/docs/Web/CSS) for help with CSS.',
'js' => '[Check the Mozilla Developer Network JS documentation](https://developer.mozilla.org/docs/Web/JavaScript) for help with JavaScript.',
'sql' => '[Check MySQL\'s official MySQL documentation](http://dev.mysql.com/doc/) for more information.',
'php' => '[Check the official PHP Documentation](http://php.net/docs.php) for help with PHP.',
Copy link
Member

Choose a reason for hiding this comment

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

'js' => '[Check the Mozilla Developer Network JS documentation](https://developer.mozilla.org/docs/Web/JavaScript) for help with JavaScript.',
'sql' => '[Check MySQL\'s official MySQL documentation](http://dev.mysql.com/doc/) for more information.',
'php' => '[Check the official PHP Documentation](http://php.net/docs.php) for help with PHP.',
'bootstrap' => '[Check the official Bootstrap Documentation](http://getbootstrap.com/getting-started/) for help with the Bootstrap framework.',
Copy link
Member

Choose a reason for hiding this comment

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

'sql' => '[Check MySQL\'s official MySQL documentation](http://dev.mysql.com/doc/) for more information.',
'php' => '[Check the official PHP Documentation](http://php.net/docs.php) for help with PHP.',
'bootstrap' => '[Check the official Bootstrap Documentation](http://getbootstrap.com/getting-started/) for help with the Bootstrap framework.',
'jquery' => '[Check the official jQuery API Documentation](http://api.jquery.com/) for help with the jQuery library.',
Copy link
Member

Choose a reason for hiding this comment

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

'html' => '[Check the Mozilla Developer Network HTML documentation](https://developer.mozilla.org/docs/Web/HTML) for help with HTML.',
'css' => '[Check the Mozilla Developer Network CSS documentation](https://developer.mozilla.org/docs/Web/CSS) for help with CSS.',
'js' => '[Check the Mozilla Developer Network JS documentation](https://developer.mozilla.org/docs/Web/JavaScript) for help with JavaScript.',
'sql' => '[Check MySQL\'s official MySQL documentation](http://dev.mysql.com/doc/) for more information.',
Copy link
Member

Choose a reason for hiding this comment

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

https://dev.mysql.com/doc/

Also note that W3S "sql" chapter is not specific to MySQL, also covers other horrifyingly shit database engines as well \o/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Indeed, I have no idea on what can be considered a good MySQL reference out there, initial searching didn't produce anything fantastic.

Copy link
Member

Choose a reason for hiding this comment

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

I think the official manual is a good place to start with anything. There's also this for specifically PDO/mysql.

…ssion matching changed to '#' from '/'.

* Metric not accurate.
@Nessworthy
Copy link
Contributor Author

Nessworthy commented Oct 5, 2016

Additional commit based on feedback. 🎆

@DaveRandom DaveRandom merged commit c7c8541 into Room-11:master Oct 6, 2016
Copy link
Contributor

@kelunik kelunik left a comment

Choose a reason for hiding this comment

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

If the RegEx fails, it should throw an exception.

@Nessworthy
Copy link
Contributor Author

Away for the weekend but I'll sort it out next week.

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