Skip to content

Conversation

thomasmarwitz
Copy link

@thomasmarwitz thomasmarwitz commented Oct 15, 2024

Add docstring_merge_delimiter as well to control the concatenation during merging docstrings.

This PR addresses #194

Todo:

  • Apply conventional commits to all commits
  • What about ✗ Checking for API breaking changes (1) => this is expected as the default config has been expanded

@thomasmarwitz thomasmarwitz marked this pull request as draft October 15, 2024 15:21
@pawamoy
Copy link
Member

pawamoy commented Oct 15, 2024

Hi @thomasmarwitz, thanks for the PR. It is however best to first discuss things in a discussion/issue before sending PRs. Are you sure you linked the right issue? #133 doesn't seem relevant to this PR.

@thomasmarwitz
Copy link
Author

Hi @thomasmarwitz, thanks for the PR. It is however best to first discuss things in a discussion/issue before sending PRs. Are you sure you linked the right issue? #133 doesn't seem relevant to this PR.

Hey @pawamoy, I meant #194, sorry for the confusion. And I'll remember for the next time :) I'm rather new to contributing to OSS. All remarks are appreciated!

@pawamoy
Copy link
Member

pawamoy commented Oct 15, 2024

OK that's what I thought (just saw #194), thanks for the clarification 🙂 So, it's not an issue per-se to open a PR without having discussed the solution first, but you just risk wasting your time working on something that won't be accepted (and that puts maintainers in an awkward position 😅).

What I would have answered on #194 is that we already support the default and if_not_present strategies suggested here. default is, well, what we do by default, and if_not_present is achieved with this Griffe extension: https://mkdocstrings.github.io/griffe/extensions/official/inherited-docstrings/. If we were to also support the merge strategy, it would happen in the Griffe extension 🙂 Would you like to move your work over there? It's possibly also easier to implement it in the extension rather than here.

We're starting to have lots of config options in mkdocstrings-python, and new ones must be added very, very carefully.

@thomasmarwitz
Copy link
Author

Thanks for your kind and explaining answer @pawamoy. I looked into the griffe extension, that seems awesome! I'll start porting my work there.

And thanks in general for providing dozens of helpful libraries 😃! Through this project, I also came in touch with duty which I didn't know before. I also really love the idea of checking for breaking changes with griffe.

@thomasmarwitz thomasmarwitz deleted the inherit_docstring branch October 16, 2024 09:34
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