Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[plotly.js] add missing plot data type #31963

Merged
merged 6 commits into from
Jan 18, 2019
Merged

[plotly.js] add missing plot data type #31963

merged 6 commits into from
Jan 18, 2019

Conversation

wacky
Copy link
Contributor

@wacky wacky commented Jan 8, 2019 •
edited
Loading

Please fill in this template.

  • 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.)
  • Add or edit tests to reflect the change. (Run with npm test.)
  • 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:

@typescript-bot
Copy link
Contributor

typescript-bot commented Jan 8, 2019 •
edited
Loading

@wacky Thank you for submitting this PR!

🔔 @chrisgervang @martinduparc @frederikaalund @taoqf @Dadstart @szechyjs @MercifulCode @soorajpudiyadath @jonfreedman @meganrm - please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

If no reviewer appears after a week, a DefinitelyTyped maintainer will review the PR instead.

@typescript-bot
Copy link
Contributor

@wacky The Travis CI build failed! Please review the logs for more information.

Once you've pushed the fixes, the build will automatically re-run. Thanks!

@typescript-bot typescript-bot added the Unmerged The author did not merge the PR when it was ready. label Jan 15, 2019
@typescript-bot
Copy link
Contributor

typescript-bot commented Jan 15, 2019 •
edited
Loading

A definition owner has approved this PR ⭐️. A maintainer will merge this PR shortly. If it shouldn't be merged yet, please leave a comment saying so and we'll wait. Thank you for your contribution to DefinitelyTyped!

jonfreedman
jonfreedman suggested changes Jan 16, 2019
View reviewed changes
types/plotly.js/index.d.ts Outdated
@@ -481,7 +481,7 @@ export type ScatterData = PlotData;
export interface PlotData {
type: 'bar' | 'box' | 'candlestick' | 'choropleth' | 'contour' | 'heatmap' | 'histogram' | 'mesh3d' |
'ohlc' | 'parcoords' | 'pointcloud' | 'scatter' | 'scatter3d' | 'scattergeo' | 'scattergl' |
'scatterternary' | 'surface';
'scatterternary' | 'surface' | 'pie' | 'scatterpolar';
Copy link
Contributor

Choose a reason for hiding this comment

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

could you re-order this to maintain the alphabetical order of chart types - it's just easier to scan

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks for reviewing. I've fixed it.

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

@wacky 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 or comments. Thank you!

@typescript-bot
Copy link
Contributor

🔔 @jonfreedman - Thanks for your review of this PR! Can you please look at the new code and update your review status if appropriate?

jonfreedman
jonfreedman approved these changes Jan 16, 2019
View reviewed changes
Copy link
Contributor

@jonfreedman jonfreedman left a comment

Choose a reason for hiding this comment

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

thanks for the PR

@typescript-bot typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Merge:Express labels Jan 16, 2019
@rbuckton rbuckton merged commit d26b035 into DefinitelyTyped:master Jan 18, 2019
@rbuckton
Copy link
Collaborator

Thanks for the contribution!

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

@jonfreedman jonfreedman jonfreedman approved these changes

Assignees
No one assigned
Labels
Owner Approved A listed owner of this package signed off on the pull request. Revision needed This PR needs code changes before it can be merged.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@wacky @typescript-bot @rbuckton @jonfreedman