Skip to content
This repository has been archived by the owner on Oct 10, 2019. It is now read-only.

Toggle unread article also now undeletes (#330) #368

Merged
merged 2 commits into from
Oct 1, 2016
Merged

Toggle unread article also now undeletes (#330) #368

merged 2 commits into from
Oct 1, 2016

Conversation

orner
Copy link
Contributor

@orner orner commented Oct 1, 2016

This fixes Issue #330, and will make sure an item is undeleted when and item is toggled unread.

Please correct me if I'm not doing this right -- I haven't contributed on Github that much. It's late Friday, I'm in a mood, so I picked one of the top issues that you had tagged.

@coveralls
Copy link

coveralls commented Oct 1, 2016

Coverage Status

Coverage decreased (-0.004%) to 28.227% when pulling 68d290f on orner:master into 7048e60 on akrennmair:master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 28.227% when pulling 68d290f on orner:master into 7048e60 on akrennmair:master.

@Minoru
Copy link
Collaborator

Minoru commented Oct 1, 2016

One of the OS X workers on the build farm froze, so I restarted it. The checks should complete any minute now.

Submission-wise there's a couple things I'd like to pick on:

  1. Please update the docs to mention the change. (Don't bother updating CHANGES file—we're keeping it in a different branch for now. Long story.)
  2. It's customary to mention issues simply by their number, just like you did in the text: toggle-article-read (N) does not undelete an article marked for deletion (D) #330. You can also use the powers of GitHub to close the issue once this PR is merged by writing "Fixes toggle-article-read (N) does not undelete an article marked for deletion (D) #330" in your commit message.

That's all. Just amend your commit and force-push an update, and I'll merge everything once checks complete. (BTW, this also means this change is going to be released with 2.10, not 2.11 as issue's milestone mention.)

@Minoru Minoru changed the title Issue 330 Toggle unread article also now undeletes Toggle unread article also now undeletes (#330) Oct 1, 2016
@orner
Copy link
Contributor Author

orner commented Oct 1, 2016

Cool! Thanks! I saw when writing the body how Github links things together.

I'll make the changes to do the docs and repush.

Concerning the unit tests, do they normally all pass? I have two test, and 7 assertions, failing, and that's working off HEAD. I think several are due to time parsing, probably since it's daylight savings (PDT) and the math is using PST, so everything is off by an hour.

@Minoru
Copy link
Collaborator

Minoru commented Oct 1, 2016

Concerning the unit tests, do they normally all pass?

Yes. We run them on Travis in a number of configurations to ensure that. It'll be cool if you could file an issue about the failing ones.

@coveralls
Copy link

coveralls commented Oct 1, 2016

Coverage Status

Coverage decreased (-0.004%) to 28.227% when pulling 3b1ff98 on orner:master into 7048e60 on akrennmair:master.

@Minoru Minoru merged commit 08db185 into akrennmair:master Oct 1, 2016
@Minoru
Copy link
Collaborator

Minoru commented Oct 1, 2016

Thank you!

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

Successfully merging this pull request may close these issues.

3 participants