Skip to content

Update docs to clarify the relationship between name and id #127

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

YgorSouza
Copy link
Contributor

The name passed to new() defines the initial id, but changing it with name() does not change the id, it has to be changed explicitly with id(). This documentation update attempts to make this caveat more explicit, as it is not apparent from the public API without looking at the source.

  • I have followed the instructions in the PR template

The name passed to new() defines the initial id, but changing it with
name() does not change the id, it has to be changed explicitly with
id(). This documentation update attempts to make this caveat more
explicit, as it is not apparent from the public API without looking at
the source.
@YgorSouza
Copy link
Contributor Author

This tripped me when I was upgrading to v0.32. I thought I could just set the new name argument to an empty string everywhere since I was already using the name() method, and only later realized the legend was partially broken because all the items had the same ID. So I thought I'd add some documentation to explain it more clearly.

Should I add a docstring to every new() method explaining that as well? I'm not sure if that's overkill.

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