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 support for `--post_date_gmt` in `post generate` #184

Merged
merged 4 commits into from Jul 13, 2018

Conversation

2 participants
@jblz
Contributor

jblz commented May 8, 2018

Sets the GMT / UTC date of the generated posts.

If the --post_date_gmt flag is provided, use that.
If not, default to the value of post_date (which defaults to the current date if it's not set).

Fixes #183

@jblz

This comment has been minimized.

Show comment
Hide comment
@jblz

jblz May 8, 2018

Contributor

Needs Tests

Contributor

jblz commented May 8, 2018

Needs Tests

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera May 15, 2018

Member

@jblz Thanks for the pull request. Are you up for providing the tests as well?

Member

schlessera commented May 15, 2018

@jblz Thanks for the pull request. Are you up for providing the tests as well?

@jblz

This comment has been minimized.

Show comment
Hide comment
@jblz

jblz May 15, 2018

Contributor

Are you up for providing the tests as well?

@schlessera Happy to, but would love a pointer on where / how to add them.

I'm not seeing anything related here for example and you can probably point me in the right direction more quickly than I can figure it out on my own :)

Contributor

jblz commented May 15, 2018

Are you up for providing the tests as well?

@schlessera Happy to, but would love a pointer on where / how to add them.

I'm not seeing anything related here for example and you can probably point me in the right direction more quickly than I can figure it out on my own :)

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera May 22, 2018

Member

@jblz The tests are done as functional tests in Behat.

You can find the current tests for wp post generate here: https://github.com/wp-cli/entity-command/blob/d44e0bfb09a63bafa5b1be0125ad619662b2f422/features/post-generate.feature

You can find an overview of how to work on tests in WP-CLI here: https://make.wordpress.org/cli/handbook/pull-requests/#running-and-writing-tests

Member

schlessera commented May 22, 2018

@jblz The tests are done as functional tests in Behat.

You can find the current tests for wp post generate here: https://github.com/wp-cli/entity-command/blob/d44e0bfb09a63bafa5b1be0125ad619662b2f422/features/post-generate.feature

You can find an overview of how to work on tests in WP-CLI here: https://make.wordpress.org/cli/handbook/pull-requests/#running-and-writing-tests

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 3, 2018

Member

@jblz Are you still up for working on the tests? Do you need further help with that?

Member

schlessera commented Jul 3, 2018

@jblz Are you still up for working on the tests? Do you need further help with that?

jblz added some commits May 3, 2018

Post Generate: Add support for post_date_gmt
Set the GMT date of the generated posts. Default: value of post_date (or current date if it's not set)
@jblz

This comment has been minimized.

Show comment
Hide comment
@jblz

jblz Jul 4, 2018

Contributor

@schlessera Thanks for the nudge (& for the instructions on getting set up w/ behat). I managed to get my test env set up & make this actually work :)

I think it's ready for review if the CI comes back green.

Contributor

jblz commented Jul 4, 2018

@schlessera Thanks for the nudge (& for the instructions on getting set up w/ behat). I managed to get my test env set up & make this actually work :)

I think it's ready for review if the CI comes back green.

Show outdated Hide outdated features/post-generate.feature
Show outdated Hide outdated features/post-generate.feature
@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 4, 2018

Member

@jblz I'm glad to hear that! The tests look good apart from a minor nitpick with the time in two of them.

And as far as I can tell, they already uncovered a bug that you needed to fix to make the tests pass, right? So it was definitely not a waste of time to write the tests.

As soon as you have done the requested changes, this will be good to be merged.

Member

schlessera commented Jul 4, 2018

@jblz I'm glad to hear that! The tests look good apart from a minor nitpick with the time in two of them.

And as far as I can tell, they already uncovered a bug that you needed to fix to make the tests pass, right? So it was definitely not a waste of time to write the tests.

As soon as you have done the requested changes, this will be good to be merged.

@jblz

This comment has been minimized.

Show comment
Hide comment
@jblz

jblz Jul 4, 2018

Contributor

they already uncovered a bug that you needed to fix to make the tests pass, right?

Yep! I needed to pull the current_time call out so we could tell if the caller was relying on defaults for post_date as well as post_date_gmt & do the correct thing in all cases.

So it was definitely not a waste of time to write the tests.

It rarely is!

Requested changes are in place.

Contributor

jblz commented Jul 4, 2018

they already uncovered a bug that you needed to fix to make the tests pass, right?

Yep! I needed to pull the current_time call out so we could tell if the caller was relying on defaults for post_date as well as post_date_gmt & do the correct thing in all cases.

So it was definitely not a waste of time to write the tests.

It rarely is!

Requested changes are in place.

@schlessera schlessera added this to the 1.3.1 milestone Jul 13, 2018

@schlessera

This comment has been minimized.

Show comment
Hide comment
@schlessera

schlessera Jul 13, 2018

Member

Thanks for the pull-request, @jblz !

Member

schlessera commented Jul 13, 2018

Thanks for the pull-request, @jblz !

@schlessera schlessera merged commit 9d3d891 into wp-cli:master Jul 13, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@schlessera schlessera changed the title from Post Generate: Add support for post_date_gmt to Add support for `--post_date_gmt` in `post generate` Jul 13, 2018

@jblz jblz deleted the jblz:patch-1 branch Jul 13, 2018

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