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

Dependency and Documentation Improvement #145

Merged
merged 12 commits into from
Aug 5, 2018

Conversation

dshanske
Copy link
Member

@dshanske dshanske commented Aug 3, 2018

This starts the process of updating the README, as well as updates the dependencies, notes PHPUnit as a composer dependency so it could be installed as part of the development package, and moves error_log to an error function inside the endpoint, opening the door for future customization. It also removes a call to an activation hook that isn't there.

@dshanske dshanske requested a review from snarfed August 3, 2018 12:16
@snarfed
Copy link
Member

snarfed commented Aug 4, 2018

woo thank you! just fyi i'm away on a brief vacation, so it may take me a day or two to get to this.

@dshanske
Copy link
Member Author

dshanske commented Aug 4, 2018

I will continue to work on PRs.

readme.md Outdated
@@ -9,7 +9,7 @@ A [Micropub](http://micropub.net/) server plugin. Available in the WordPress plu

Once you've installed and activated the plugin, try using [Quill](http://quill.p3k.io/) to create a new post on your site. It walks you through the steps and helps you troubleshoot if you run into any problems. After that, try other clients like [OwnYourGram](http://ownyourgram.com/), [OwnYourCheckin](https://ownyourcheckin.wirres.net/), [MobilePub](http://indiewebcamp.com/MobilePub), and [Teacup](https://teacup.p3k.io/).

Supports the [full W3C Micropub CR spec](https://www.w3.org/TR/micropub/) as of 2016-10-18, except for the optional media endpoint. Media may be uploaded directly to the wordpress-micropub endpoint as multipart/form-data, or sideloaded from URLs.
Supports the [full W3C Micropub CR spec](https://www.w3.org/TR/micropub/) as of version 2.0.0
Copy link
Member

Choose a reason for hiding this comment

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

very minor nit: needs a period at the end.

Copy link
Member

@snarfed snarfed left a comment

Choose a reason for hiding this comment

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

woo! thank you for docs! i don't have much (any) experience with travis or phpcs, so i'm pretty useless reviewing this. all i have is a typo. :P hey @pfefferle, any chance you could take a look?

@dshanske
Copy link
Member Author

dshanske commented Aug 4, 2018

@snarfed The update actually revealed some mistakes I made in the tests, which made it worth doing. It also now tests 7.2 and 5.4, which makes sense considering the plugin says it supports 5.3 and above. The phpcs update is because the WordPress Coding standard finally hit 1.0 and the VIP standards are merged in. So, I have to do that for all my projects.

@dshanske dshanske requested a review from pfefferle August 5, 2018 04:46
@@ -49,8 +49,18 @@ public static function return_micropub_error( $response, $handler, $request ) {
return $response;
}



public static function log_error( $message, $name = '' ) {
Copy link
Member

Choose a reason for hiding this comment

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

If you set 'Micropub' as default for $name, you do not have to check in line 59.

$message = wp_json_encode( $message );
}
if ( ! is_string( $name ) || empty( $name ) ) {
$name = 'Micropub: ';
Copy link
Member

Choose a reason for hiding this comment

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

Replace $name = 'Micropub: '; with $name = 'Micropub'; becaue the : is set by the error logger in line 62

@pfefferle
Copy link
Member

Travis and PHPCS looks good to me... @dshanske Have you thought about an auto-releaser, like we use for the other IndieWeb plugins?

@dshanske
Copy link
Member Author

dshanske commented Aug 5, 2018

@pfefferle I thought about an autoreleaser. I also use a push script for several plugins as well. I figured I'd defer to snarfed.

@dshanske
Copy link
Member Author

dshanske commented Aug 5, 2018

If he's interested, I'd do it.

@dshanske dshanske merged commit e698894 into indieweb:master Aug 5, 2018
dougbeal pushed a commit to dougbeal/wordpress-micropub that referenced this pull request Aug 15, 2018
* master:
  Fix php notices (indieweb#149)
  Move permissions to be checked before all other actions (indieweb#148)
  Dependency and Documentation Improvement (indieweb#145)
  Introduce REST API endpoint (indieweb#143)
@dshanske dshanske deleted the dependency branch July 25, 2020 04:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants