-
Notifications
You must be signed in to change notification settings - Fork 153
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
docs: describe how to use with non-serializable options #252
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please hard wrap at ~80 characters to be consistent with the rest of the docs.
@jsumners the rest of the docs by no means comply with this arbitrary 80 character limit. In any case, if you really want to enforce that, the best way would be to get one of the automated checks to fail. |
I expanded the first available expansion below the change in the GitHub diff and it showed the hard wrapping. I'd rather follow up with a "fix" on the rest of the docs later than continue the inconsistency here. |
I'm sorry @jsumners, not trying to start an argument here but if you want certain standards in the codebase it's up to you to enforce them. You're using standardjs, which has no concept of maximum line length and you have nothing in place to enforce that. You can easily see for yourself that the Readme file doesn't comply with any length limits and I have no easy way to format it that way simply because the tooling that's in the repository doesn't allow me to do that. So basically what you're asking me is that I manually go and break all lines to a maximum of 80 characters, which to be honest is not something which is fair to expect from any contributor unless the repository provides an easy way to do that. |
Again, when I expand the diff in this PR, I can see that the rule is followed in at least the very next documentation section: If you're using VSCode as your editor, you can can use https://marketplace.visualstudio.com/items?itemName=stkb.rewrap. Select the text to be wrapped and press I'm not requesting that you fix the rest of the docs, if they are indeed out of order. I am only asking that we not perpetuate the inconsistency. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Closes #242