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

Added priority feature from issues. #2

Merged
merged 18 commits into from
Aug 11, 2024
Merged

Conversation

vhespanha
Copy link

Hey!

I've just added the feature you suggested in the repo issues. From my testing, it seems to be working flawlessly. If you spot any problems with the implementation, I'd be more than happy to fix them.

The diffs include quite a few changes related to formatting and import order. My editor automatically formatted a lot of the files. I felt these changes weren't bad to the repo, so I kept them in. However, if you'd prefer, I can reopen this PR without these formatting changes.

If this gets merged, I'd love to work on some other chores for the repo, like implementing a Makefile similar to the build script you have, along with other stuff that could be automated.

Thanks!

@dynnian
Copy link
Owner

dynnian commented Aug 11, 2024

Thank you for the PR! For my brief testing and some review It's working great. The only thing that still needs to be updated is the help page. If you can add this to finish the PR, I will merge it immediately. Otherwise I will merge it anyway as is and later when I have the time, update it myself.

@vhespanha
Copy link
Author

Thank you!!

I'll update it first thing in the morning so we can close it.

@vhespanha
Copy link
Author

Made some more changes besides what you suggested, but here it is!

The help command was using cli-todo instead of clido so i changed it, but if this was intended i can revert it.

@vhespanha
Copy link
Author

Totally unrelated to this PR, but I'm gonna open a draft of this same CLI using cobra, adding another external dependency might not be what you aim for with the project, but I'll do it so you can take a look at it and decide if you like the idea or not.

@dynnian
Copy link
Owner

dynnian commented Aug 11, 2024

Thanks!

The help command was using cli-todo instead of clido so i changed it, but if this was intended i can revert it.

Nope, that was totally a mistake. cli-todo was a previous name of this tool.

Totally unrelated to this PR, but I'm gonna open a draft of this same CLI using cobra, adding another external dependency might not be what you aim for with the project, but I'll do it so you can take a look at it and decide if you like the idea or not.

Cobra library looks great, I think it's a great idea.

@dynnian
Copy link
Owner

dynnian commented Aug 11, 2024

Merging!

@dynnian dynnian merged commit 268e074 into dynnian:main Aug 11, 2024
@dynnian
Copy link
Owner

dynnian commented Aug 11, 2024

Totally unrelated to this PR, but I'm gonna open a draft of this same CLI using cobra, adding another external dependency might not be what you aim for with the project, but I'll do it so you can take a look at it and decide if you like the idea or not.

Add an issue with this as a draft. For a next release we can have this, alongside priority and json output.

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