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

changes js code to ts in region_map #2084

Merged
merged 2 commits into from
Aug 9, 2022
Merged

changes js code to ts in region_map #2084

merged 2 commits into from
Aug 9, 2022

Conversation

Shivamdhar
Copy link
Contributor

@Shivamdhar Shivamdhar commented Aug 5, 2022 •
edited
Loading

Signed-off-by: Shivam Dhar [email protected]

Description

No new functionality added.

Issues Resolved

#2039

Check List

  • New functionality includes testing.
    • All tests pass
      • yarn test:jest
      • yarn test:jest_integration
      • yarn test:ftr
  • New functionality has been documented.
  • Commits are signed per the DCO using --signoff

@Shivamdhar Shivamdhar requested a review from a team as a code owner August 5, 2022 18:15
@AMoo-Miki AMoo-Miki requested a review from kavilla August 5, 2022 18:18
junqiu-lei
junqiu-lei previously approved these changes Aug 5, 2022
View reviewed changes
kavilla
kavilla reviewed Aug 5, 2022
View reviewed changes
Copy link
Member

@kavilla kavilla left a comment

Choose a reason for hiding this comment

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

Thank you so much for doing this a couple more questions. I still want to pull down but are there any typescript issues?

src/plugins/region_map/server/plugin.ts Outdated Show resolved Hide resolved
src/plugins/region_map/server/services/index.ts Outdated Show resolved Hide resolved
src/plugins/region_map/server/plugin.ts Outdated Show resolved Hide resolved
@Shivamdhar Shivamdhar requested a review from kavilla August 5, 2022 20:13
joshuarrrr
joshuarrrr previously requested changes Aug 5, 2022
View reviewed changes
Copy link
Member

@joshuarrrr joshuarrrr left a comment

Choose a reason for hiding this comment

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

A couple minor questions and changes requested.

src/plugins/region_map/server/plugin.ts Outdated Show resolved Hide resolved
src/plugins/region_map/server/plugin.ts Outdated Show resolved Hide resolved
src/plugins/region_map/server/services/opensearch_service.ts Outdated Show resolved Hide resolved
src/plugins/region_map/server/plugin.ts Outdated Show resolved Hide resolved
@kavilla
Copy link
Member

kavilla commented Aug 5, 2022

https://github.com/opensearch-project/OpenSearch-Dashboards/blob/main/src/plugins/region_map/server/routes/opensearch.ts#L9 are implicitly typed as any which would be a TypeScript issue can we explicitly type them.

@ananzh ananzh added refactor Tech debt related tasks that need refactoring backport 2.2 labels Aug 8, 2022
@ananzh ananzh changed the title changes js code to ts changes js code to ts in region_map Aug 8, 2022
@ananzh ananzh added v2.2.0 maps Issues or PRs related to the Maps Service labels Aug 8, 2022
@Shivamdhar
changes js code to ts
e890f1e 
Signed-off-by: Shivam Dhar <[email protected]>
ashwin-pc
ashwin-pc previously approved these changes Aug 8, 2022
View reviewed changes
junqiu-lei
junqiu-lei previously approved these changes Aug 9, 2022
View reviewed changes
mihirsoni
mihirsoni previously approved these changes Aug 9, 2022
View reviewed changes
Copy link
Contributor

@mihirsoni mihirsoni left a comment

Choose a reason for hiding this comment

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

LGTM !! Run prettier / eslint to meet the style guideline.

kavilla
kavilla previously approved these changes Aug 9, 2022
View reviewed changes
@Shivamdhar
removes unused imports
0d76e54 
Signed-off-by: Shivam Dhar <[email protected]>
@Shivamdhar Shivamdhar dismissed stale reviews from kavilla, mihirsoni, junqiu-lei, and ashwin-pc via 0d76e54 August 9, 2022 18:15
@kavilla kavilla merged commit 45324d3 into opensearch-project:main Aug 9, 2022
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 9, 2022
@Shivamdhar @github-actions
changes js code to ts in region_map (#2084)
3cb20d7 
Issue:
#2039

* changes js code to ts and use the updated client

Signed-off-by: Shivam Dhar <[email protected]>
(cherry picked from commit 45324d3)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Aug 9, 2022
Merged
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 9, 2022
@Shivamdhar @github-actions
changes js code to ts in region_map (#2084)
b8e8d39 
Issue:
#2039

* changes js code to ts and use the updated client

Signed-off-by: Shivam Dhar <[email protected]>
(cherry picked from commit 45324d3)
@opensearch-trigger-bot opensearch-trigger-bot bot mentioned this pull request Aug 9, 2022
Merged
kavilla pushed a commit that referenced this pull request Aug 9, 2022
@opensearch-trigger-bot @Shivamdhar
changes js code to ts in region_map (#2084) (#2103)
06a5338 
Issue:
#2039

* changes js code to ts and use the updated client

Signed-off-by: Shivam Dhar <[email protected]>
(cherry picked from commit 45324d3)

Co-authored-by: Shivam Dhar <[email protected]>
kavilla pushed a commit that referenced this pull request Aug 9, 2022
@opensearch-trigger-bot @Shivamdhar
changes js code to ts in region_map (#2084) (#2102)
61090ea 
Issue:
#2039

* changes js code to ts and use the updated client

Signed-off-by: Shivam Dhar <[email protected]>
(cherry picked from commit 45324d3)

Co-authored-by: Shivam Dhar <[email protected]>
@kavilla kavilla linked an issue Aug 12, 2022 that may be closed by this pull request
[BUG] Region Map Plug JS code #2039
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ananzh ananzh ananzh approved these changes

@kavilla kavilla kavilla approved these changes

@junqiu-lei junqiu-lei junqiu-lei left review comments

@ashwin-pc ashwin-pc ashwin-pc left review comments

@mihirsoni mihirsoni mihirsoni left review comments

@joshuarrrr joshuarrrr Awaiting requested review from joshuarrrr joshuarrrr is a code owner

Assignees

@Shivamdhar Shivamdhar

Labels
backport 2.x maps Issues or PRs related to the Maps Service refactor Tech debt related tasks that need refactoring v2.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Region Map Plug JS code
7 participants
@Shivamdhar @kavilla @mihirsoni @joshuarrrr @ashwin-pc @ananzh @junqiu-lei