Skip to content

Conversation

0styx0
Copy link
Contributor

@0styx0 0styx0 commented Jan 24, 2022

This pr builds on and supersedes #323 to implement full category support using the -c/—category flag.
Solves #10

Copy link
Member

@WhyNotHugo WhyNotHugo left a comment

Choose a reason for hiding this comment

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

Thanks for addressing this, this is a pretty tricky one.

I think you're not deleting categories from the categories table when a todo is deleted -- and they might show up for later todos when ids get flushed.

@0styx0
Copy link
Contributor Author

0styx0 commented Jan 31, 2022

Thanks for addressing this, this is a pretty tricky one.

I think you're not deleting categories from the categories table when a todo is deleted -- and they might show up for later todos when ids get flushed.

the line FOREIGN KEY(todos_id) REFERENCES todos(id) ON DELETE CASCADE in https://github.com/0styx0/todoman/blob/b2063c84fbcd0fb84d3bb540e661e40f6b505d3b/todoman/model.py#L506 should take care of that. I added a test in 004d812 to make sure, let me know if I missed it though

@0styx0 0styx0 requested a review from WhyNotHugo February 1, 2022 17:00
adding the [] check to pass tests/test_cli.py::test_edit_move
@0styx0 0styx0 requested a review from WhyNotHugo February 13, 2022 20:33
Copy link
Member

@WhyNotHugo WhyNotHugo left a comment

Choose a reason for hiding this comment

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

LGTM. Sorry for the delay, lost sight of this.

@WhyNotHugo WhyNotHugo merged commit 4c1c6a7 into pimutils:main Jul 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants