I like the look of the groups, but it hides --subscribe, the only real option, because those groups standout more. Maybe if subscribe was in a group...

I definitely will be using --close-after-startup to benchmark attempts at reducing startup time. However, I'm not sure if it should ship with gpodder, and if it did, only the long option name should be available in case another option needs the -c later on, but also to make it harder to accidentally use.

But if added, it might be good to call self.core.db.close() before exiting.

@elelay What do you think?

@elelay What do you think?

agreed with all your comments

Patches updated,

• --subscribe is in a group now so it stands out more
• short option name -c is removed
• self.core.db.close is called before exit

New help output:

$ bin/gpodder -h
Usage: gpodder [options]

gPodder enables you to subscribe to media feeds (RSS, Atom, YouTube,
Soundcloud and Vimeo) and automatically download new content.

This is the gPodder GUI. See gpo(1) for the command-line interface.

Options:
  --version             show program's version number and exit
  -h, --help            show this help message and exit

  Subscriptions:
    -s URL, --subscribe=URL
                        subscribe to the feed at URL

  Logging:
    -v, --verbose       print logging output on the console
    -q, --quiet         reduce warnings on the console

  Advanced:
    --close-after-startup
                        exit once started up (for profiling)

However, I'm not sure if it should ship with gpodder, [...]

Personally I'd find it useful. It would allow to use this from any branch without cherry picking the patch(es) first, and it would make it available in distro packages (I'll probably test some time soon how much I can reduce startup time by not installing all plugins at once - in Alpine Linux / postmarketOS it's not uncommon to split out plugins into extra packages since the overhead is very small). So for that it it would be helpful too if the patch was in the main repository.

But in the end it's your (the maintainers) call, and if you decide that you'd rather not have these two patches in master then that's also fine. 👍

Cool that there's a linter in CI now! however the failure seems unrelated? it just says Error: (or am I reading it wrong?)

I've tried to follow PEP-8 in the patches.

Cool that there's a linter in CI now! however the failure seems unrelated? it just says Error: (or am I reading it wrong?)

I've tried to follow PEP-8 in the patches.

It shows it in the output: https://github.com/gpodder/gpodder/pull/1108/checks?check_run_id=3212955349 and even suggests a fix:

-from optparse import OptionParser, OptionGroup
+from optparse import OptionGroup, OptionParser

The first patch is a good addition. It is the second patch that has very limited use and could be done by adding two lines into the source when needed.

Can the groups be rearranged? If so, do you think Logging(top), Advanced(middle), Subscriptions(bottom) would be better?

They can be re-arranged, I could put them in the suggested order. However, shouldn't we put the most important group on top? I'd expect users to read the help output from top to bottom (and this is how I read it myself) :)

It is the second patch that has very limited use and could be done by adding two lines into the source when needed.

Another nice thing would be that people could replicate the startup time test easily, without the issue of possibly inserting lines at the wrong place and therefore doing a different measurement.

I think if it's made easy to test the startup time, it's more likely that people will keep an eye on it.

merged, thanks!