Trevoke / org-gtd.el

A package for using GTD with org-mode
GNU General Public License v3.0
376 stars 46 forks source link

Rename org-gtd-review.el to org-gtd-reflect.el #168

Open doolio opened 1 year ago

doolio commented 1 year ago

This renaming aligns it with the name for step 4 of the GTD methodology. All commands renamed accordingly.

Resolves: #164

doolio commented 1 year ago

I believe the order of commands/functions is how you expect to see them so no changes required in that regard.

Trevoke commented 1 year ago

There are conflicts because of the latest bugfix I merged in (which gitignores the autoloads file, among other things).

Here's the fixes that I think we need to apply / open questions left to answer:

doolio commented 1 year ago

please use version numbers and not dates in the make-obsolete statements (e.g. "3.1.0", since that's the next version release)

OK.

you don't need to write this much for the second argument of make-obsolete since you can just put the symbol for the new function name (at least I'm 90% sure that (make-obsolete 'org-gtd-review-stuck-calendar-items 'org-gtd-reflect-stuck-calendar-items "3.1.0") will work as needed, but I could be wrong)

OK, I simply followed the ~example~ suggestion in the make-obsolete docstring as it also updates the related docstring. I'll look to see what you suggest does to the docstring if anything.

Is this the right way to set up the autoloads?

I included an autoload cookie as the example in the docstring included one I believe or I saw it being included in other org files when a command is being made obsolete.

Shouldn't the functions have autoload cookies as well?

Well, yes (and I had those in mind in raising #156 but as they didn't originally I didn't give them one in this PR.

Did you test this to make sure both old and new functions properly loaded and executed without a require statement?

No, but I will now that you have removed the autoloads file.

make sure you are only changing NEW documentation, nothing in the past releases (this means "changes for 3.0" can't be touched, and none of the "upgrade path" sections can be touched either)

OK, I'll undo what I have done in those sections.

the test/reviews-test.el should also be changed to test/reflect-test.el

OK, thanks.

Trevoke commented 1 year ago

This looks good, thank you! Tell me about the autoloads? What did you learn?