-
Notifications
You must be signed in to change notification settings - Fork 163
fixed var name #78
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
fixed var name #78
Conversation
Good catch. Can you remove the changes to lib/? We'll make those when we rebuild for release.
@taion all set
LTGM
Actually, wait a minute. The tests cover the rendering. The reason this didn't cause any failures is that this method is never used.
In principle this could be used by custom components, but it looks like LinkMixin is not intended to be a public API, as it's not documented and also not exported here: https://github.com/react-bootstrap/react-router-bootstrap/blob/b2de57c5c79bf73736483e5805267121533eb045/src/index.js
I'm wondering if we should delete this method altogether. Nevertheless that's not a question for this PR.
LGTM
Yeah. It is not used and I'm removing it in #76
here https://github.com/react-bootstrap/react-router-bootstrap/pull/76/files#diff-b51e6bdcefbce91ad1e13b562ee26d15L43 😄
But nobody wants to review it and merge so far 😜
Oh, I thought we already merged #76. That supersedes this request, then. Apologies for having made you do the extra work, @kriswallsmith.
Looks like this var name was missed in the refactor.