Skip to content

Conversation

@KurtPreston
Copy link
Contributor

  • Use a meaningful title for the pull request. Include the name of the package modified.
  • Test the change in your own code. (Compile and run.)
  • Follow the advice from the readme.
  • Avoid common mistakes.
  • Run npm run lint package-name (or tsc if no tslint.json is present).

If changing an existing definition:

  • Provide a URL to documentation or source code which provides context for the suggested changes: https://reactstrap.github.io/components/carousel/
  • Increase the version number in the header if appropriate.
  • If you are making substantial changes, consider adding a tslint.json containing { "extends": "dtslint/dt.json" }.

@typescript-bot
Copy link
Contributor

After 5 days, no one has reviewed the PR 😞. A maintainer will be reviewing the PR in the next few days and will either merge it or request revisions. Thank you for your patience!

@typescript-bot typescript-bot added the Unmerged The author did not merge the PR when it was ready. label Dec 31, 2017
export { default as Table, TableProps } from './lib/Table';
export { default as TabPane, TabPaneProps } from './lib/TabPane';
export { default as Tag, TagProps } from './lib/Tag';
export { default as TetherContent, TetherContentProps } from './lib/TetherContent';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removal of tether is a breaking change in v 5-alpha.

First, the version should be updated to reflect this, see https://github.com/DefinitelyTyped/DefinitelyTyped#i-want-to-update-a-package-to-a-new-major-version since it is a major version.

Second, consider nod making the breaking change untill a stable release is available.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, my bad.

@typescript-bot typescript-bot added Revision needed This PR needs code changes before it can be merged. and removed Unmerged The author did not merge the PR when it was ready. labels Jan 2, 2018
@typescript-bot
Copy link
Contributor

@KurtPreston One or more reviewers has requested changes. Please address their comments. I'll be back once they sign off or you've pushed new commits. Thank you!

@KurtPreston
Copy link
Contributor Author

@mhegazy The type definitions have already been bumped to v5-alpha, since commit f7f81bc two months ago. There is a separate v4 folder inside the type definition as per the document you referenced.

@LKay
Copy link
Contributor

LKay commented Jan 3, 2018

@KurtPreston Could you also add tag: React.ReactType to props definition for Col? It's a oneliner and that seems to be missing.

@mhegazy mhegazy merged commit 52bc966 into DefinitelyTyped:master Jan 3, 2018
@KurtPreston
Copy link
Contributor Author

@mhegazy Thanks!

@LKay See #22655

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Revision needed This PR needs code changes before it can be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants