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/Remove Dates When Publishing/Unpublishing #92

Merged
merged 7 commits into from
Apr 4, 2019
Merged

Add/Remove Dates When Publishing/Unpublishing #92

merged 7 commits into from
Apr 4, 2019

Conversation

gavinhoward
Copy link
Contributor

@gavinhoward gavinhoward commented Apr 4, 2019

My second attempt to fix dates when publishing and unpublishing. The first attempt was #91, and it had several problems. I believe that this PR fixes the problems with #91 that I was told about; if not, please let me know.

I took the time to learn how to use git merge better, so after some work, I was able to keep @alzeih's commit (and authorship) intact.

I hope that it was okay to put myself as the author on the other commits, though.

Anyway, here are the changes I made:

  1. I learned how to use RSpec and introduced the tests that were missing.
  2. I fixed the RuboCop offenses listed by /script/fmt, though I did fix each one manually (not with /script/fmt -a).
  3. I made sure that dates are completely removed when unpublishing a post.

This commit will close #55 and will also resolve #63.

@wip ready for review

@DirtyF DirtyF requested a review from a team April 4, 2019 16:33
@DirtyF DirtyF added the fix label Apr 4, 2019
@DirtyF DirtyF changed the title Add/Remove Dates When Publishing/Unpublishing (Attempt 2) Add/Remove Dates When Publishing/Unpublishing Apr 4, 2019
@ashmaroli
Copy link
Member

@gavinhoward Good Job with learning the new stuff.
The commits are proper this time around. However, I'll recommit the changes with you as co-author so that you'll get due credit when our merge-bot merges this PR. (It squashes all commits automatically prior to the merge.)

@gavinhoward
Copy link
Contributor Author

@ashmaroli thank you. If it helps, I feel no particular need to be credited.

Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

Needs some changes to be made..

lib/jekyll-compose/file_mover.rb Outdated Show resolved Hide resolved
lib/jekyll-compose/file_mover.rb Outdated Show resolved Hide resolved
lib/jekyll-compose/file_mover.rb Outdated Show resolved Hide resolved
lib/jekyll-compose/file_mover.rb Outdated Show resolved Hide resolved
lib/jekyll/commands/publish.rb Outdated Show resolved Hide resolved
lib/jekyll/commands/unpublish.rb Outdated Show resolved Hide resolved
@DirtyF DirtyF requested a review from ashmaroli April 4, 2019 20:21
Copy link
Member

@ashmaroli ashmaroli left a comment

Choose a reason for hiding this comment

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

LGTM! But would help to get a second opinion since the recent changes are my suggestions

@ashmaroli ashmaroli requested a review from a team April 4, 2019 20:34
Copy link
Member

@DirtyF DirtyF left a comment

Choose a reason for hiding this comment

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

Nice work @gavinhoward 👍

@DirtyF
Copy link
Member

DirtyF commented Apr 4, 2019

@jekyllbot: merge +minor

@jekyllbot jekyllbot merged commit ef25d41 into jekyll:master Apr 4, 2019
jekyllbot added a commit that referenced this pull request Apr 4, 2019
@DirtyF
Copy link
Member

DirtyF commented Apr 4, 2019

@gavinhoward 📆 shipped in /~https://github.com/jekyll/jekyll-compose/releases/tag/v0.11.0 🎉

@gavinhoward
Copy link
Contributor Author

Yes! \o/

@jekyll jekyll locked and limited conversation to collaborators Apr 4, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add date to front matter when publishing draft
5 participants