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.

Sign up for GitHub

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

Jump to bottom

Listing all entries in DayOne Classic journal throws IndexError #786

Merged
merged 4 commits into from
Feb 8, 2020
Merged

Listing all entries in DayOne Classic journal throws IndexError #786

merged 4 commits into from
Feb 8, 2020

Conversation

MinchinWeb
Copy link
Contributor

@MinchinWeb MinchinWeb commented Jan 5, 2020 •
edited by wren
Loading

Closes #780

This re-creates the issue in #780 and then fixes it. This is caused by a DayOne entry without any body text. That said, there is nothing DayOne particular about it, so in theory it could happen with a "regular" text jrnl file.

Checklist

  • The code change is tested and works locally.
  • Tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • Have you followed the guidelines in our Contributing document?
  • Have you checked to ensure there aren't other open Pull Requests for the same update/change?
  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your core changes, as applicable? -- Yup!!

@MinchinWeb
Reproduce bug in jrnl-org#780
0c00491 
🎵 I have no body, no body to love me... 🎵

The bug is cause by a DayOne entry that has to entry body.
@MinchinWeb
Deal with empty bodies
baeb8c1 
Close jrnl-org#780.
@MinchinWeb MinchinWeb mentioned this pull request Jan 5, 2020
Closed
@MinchinWeb
[Travis-CI] add "tree" command to debug missing files
50c1347
@MinchinWeb
Fix file location
7d3895b 
I have no idea why, but it ran locally fine without issue. Travis is more particular...
@MinchinWeb
Copy link
Contributor Author

So the tests should pass once PR #785 (DayOne UTC support) is merged.

wren
wren requested changes Jan 17, 2020
View reviewed changes
Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

I know this isn't ready due to failing tests, so I'm just marking it like this to get it out of my review queue for now. Please just let me know when it's ready for review.

@MinchinWeb MinchinWeb mentioned this pull request Jan 17, 2020
Merged
7 tasks
@wren wren changed the title Bug780 Listing all entries in DayOne Classic journal throws IndexError Feb 8, 2020
@wren wren added the bug Something isn't working label Feb 8, 2020
wren
wren approved these changes Feb 8, 2020
View reviewed changes
Copy link
Member

@wren wren left a comment

Choose a reason for hiding this comment

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

🥇

@wren wren merged commit e5d7845 into jrnl-org:develop Feb 8, 2020
@MinchinWeb MinchinWeb deleted the bug780 branch March 21, 2020 22:56
wren pushed a commit that referenced this pull request Apr 18, 2020
@MinchinWeb
Listing all entries in DayOne Classic journal throws IndexError (#786)
cec0bef 
* Reproduce bug in #780
🎵 I have no body, no body to love me... 🎵
The bug is cause by a DayOne entry that has to entry body.

* Deal with empty bodies
Close #780.

* [Travis-CI] add "tree" command to debug missing files
* Fix file location
I have no idea why, but it ran locally fine without issue. Travis is more particular...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wren wren wren approved these changes

Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[DayOne] Listing all entries throws IndexError
2 participants
@MinchinWeb @wren