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

Add 'Keep' option #21

Closed
petenelson opened this issue Jun 6, 2016 · 13 comments
Closed

Add 'Keep' option #21

petenelson opened this issue Jun 6, 2016 · 13 comments

Comments

@petenelson
Copy link
Contributor

Already working on a PR for this, but it would be handy to be able to keep a minimum number of revisions for a post. In some instances, it's handy to be able to look at post that has passed the days back setting and still be able to see at least the past couple of revisions.

@stevegrunwell
Copy link
Owner

I'll look forward to the PR. Same use-case that @jonbellah was pinging me about (a certain client project)?

I'm guessing this probably won't be default behavior, but adding some well-placed filters to alter the query that collects eligible revision IDs would probably be a good starting point ;)

@petenelson
Copy link
Contributor Author

It's done, just trying to get the unit tests working now. It's on my fork of the repo in a new branch if you want to take a look before the PR.

@petenelson
Copy link
Contributor Author

Any ideas? Haven't modified any of the unit tests but it's failing: https://travis-ci.org/petenelson/revision-strike/jobs/135712895

@stevegrunwell
Copy link
Owner

@petenelson There were apparently some version-related issues affecting earlier builds of Revision Strike (that actually let everything pass). I've done some refactoring in the fix/travis-ci branch, which has been merged into develop (the new base branch, as of a couple of minutes ago).

Please go ahead and merge develop into your local branch, at which point the tests should be passing (barring any errors resulting from code you've changed). Then, when you open your PR, please open it against the develop branch. Once merged, I'll get a new release out to unblock you and your team :)

@stevegrunwell stevegrunwell added this to the Version 0.3 milestone Jun 10, 2016
@petenelson
Copy link
Contributor Author

Haven't made any changes to unit tests, build is still failing: https://travis-ci.org/petenelson/revision-strike/builds/136721782

@stevegrunwell
Copy link
Owner

That's likely because there have been changes to the codebase that aren't being reflected in the codebase. The test suite for Revision Strike makes heavy use of WP_Mock to verify that, for example, get_options() is being called with the expected arguments + output. As opposed to core-style "unit" testing (which is really closer to integration testing), this test suite tests each function/method independently of one another; it's more setup than the stock WordPress test suite, but it allows me to verify that every function/method is working as expected.

I'd recommend ensuring that everything's working locally (via ./vendor/bin/phpunit) rather than having to push each change to Travis (which takes a while to run); from the output of the Travis run, it looks like there are quite a few get_option() calls that aren't being mocked, causing your tests to fail.

Also, as an aside, please make sure you're adding DocBlocks to your apply_filters() calls, per the WordPress Inline Documentation Standards.

@petenelson
Copy link
Contributor Author

I pushed your develop branch up and it's failing https://travis-ci.org/petenelson/revision-strike/jobs/136728393

I don't have PHP 5.6 installed locally yet but will get my local VVV updated.

@stevegrunwell
Copy link
Owner

It's failing due to a lack of the $CODECLIMATE_REPO_TOKEN environment variable, which needs to be set up in the Travis-CI configuration. It doesn't really make sense for forks of the repo to include it, but [Code Climate](https://codeclimate.com/github/stevegrunwell/revision-strike helps validate my coding style and track test coverage).

I just pushed a new version to the develop branch that wraps the test-reporter call in a conditional, so it will only execute on environments that have $CODECLIMATE_REPO_TOKEN configured. Pushing develop to Travis CI should now generate passing tests on your account.

@petenelson
Copy link
Contributor Author

petenelson commented Jun 11, 2016

Working on getting unit tests installed. The readme says to run $ bin/install-package-tests.sh but by itself, it fails unless I temporarily add WP_CLI_BIN_DIR=/tmp/wp-cli-phar to the .sh file. Not sure if I was doing something wrong.

I have the develop branch tests working locally, but not the tests on the changes yet. Trying to figure that out, haven't worked with WP_Mock yet, so may have questions on that.

@petenelson
Copy link
Contributor Author

PHPUnit 5.4.4 by Sebastian Bergmann and contributors.

Runtime:       PHP 5.6.22-1+donate.sury.org~trusty+1
Configuration: /srv/www/wordpress-default/wp-content/plugins/revision-strike/phpunit.xml.dist
Error:         No code coverage driver is available

......PHP Fatal error:  Call to undefined function get_option() in /srv/www/wordpress-default/wp-content/plugins/revision-strike/includes/class-settings.php on line 171

@petenelson
Copy link
Contributor Author

Success! https://travis-ci.org/petenelson/revision-strike/builds/137527366

I'm also going to add an option to the tools/settings pages to configure the post types. The project this update is for has most of its revisions on pages, so we'll need the option to clean revisions for pages in the UI.

@petenelson
Copy link
Contributor Author

Scratch that, will use the existing revisionstrike_post_types filter in our own plugin due to time constraints with the project, and will work on making it configurable in a future version. PR coming shortly.

@stevegrunwell
Copy link
Owner

Per our discussion in #22, this should now be possible via the revisionstrike_get_revision_ids filter added v0.3.0.

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

No branches or pull requests

2 participants