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.

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

2.0 version bump #230

Merged
merged 3 commits into from
Apr 7, 2022
Merged

2.0 version bump #230

merged 3 commits into from
Apr 7, 2022

Conversation

amitgalitz
Copy link
Member

@amitgalitz amitgalitz commented Apr 6, 2022

Description

  1. Version bumped 1.3 to 2.0
  2. added test environment that is now required for jest
  3. removed jest-dom and react dependency in AD as we are now using a more up to date version from OSD
  4. change wait() call to waitFor()
  5. AnnotationDomainTypes changed to AnnotationDomainType
  6. typescript upgrade made typing more restrictive so had to delete a line in get request which would make the body for get request undefined rather than null
  7. Changed SampleDataBox component to use EuiCard instead of ContentPanel. This is because betaBadgeLabel was removed from EuiPanel which is used in ContentPanel component. This required a few changes in padding and adding a horizontal line below title manually.
  8. Changed SampleDataBox to display in a grid so cards don't come off screen when minimizing it.
  9. updated integ and unit test workflow to 2.0 and main branch on OSD

Issues Resolved

resolves #221
resolves #222
resolves #164

Check List

  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

@amitgalitz amitgalitz requested a review from a team April 6, 2022 18:14
@opensearch-trigger-bot opensearch-trigger-bot bot added backport 1.x dependencies Pull requests that update a dependency file labels Apr 6, 2022
@amitgalitz amitgalitz mentioned this pull request Apr 6, 2022
1 task
@amitgalitz amitgalitz added v2.0.0 Version 2.0.0 breaking and removed backport 1.x labels Apr 6, 2022
@codecov-commenter
Copy link

codecov-commenter commented Apr 6, 2022

Codecov Report

Merging #230 (d92c6f1) into main (39086ba) will increase coverage by 0.02%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main     #230      +/-   ##
==========================================
+ Coverage   45.90%   45.92%   +0.02%     
==========================================
  Files         150      150              
  Lines        4956     4956              
  Branches      955      968      +13     
==========================================
+ Hits         2275     2276       +1     
  Misses       2463     2463              
+ Partials      218      217       -1     
Impacted Files Coverage Δ
...in/public/components/ContentPanel/ContentPanel.tsx 62.50% <0.00%> (ø)
...s/Overview/containers/AnomalyDetectionOverview.tsx 32.92% <0.00%> (ø)
...verview/components/SampleDataBox/SampleDataBox.tsx 77.77% <0.00%> (ø)
...onfirmActionModals/ConfirmDeleteDetectorsModal.tsx 85.29% <0.00%> (+2.94%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39086ba...d92c6f1. Read the comment docs.

</div>
</div>
<div
aria-hidden="true"
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems we changed a lot for alerts flyout. Looks quite simple now. Can you confirm if the AlertsFlyout snap is correct or not?

Copy link
Member Author

Choose a reason for hiding this comment

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

Investigating this now, ConfirmModal snap also had a lot removed. Need to see why testing isn't opening flyout/modal.

Copy link
Member Author

Choose a reason for hiding this comment

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

Still no clear answer on this. I opened an issue: #231 and will continue investigating with the help on others on dashboard team. For the failing integ-test, the correct UI is still there but for some reason the aria-label for "Daily max" isn't present in the element. I will make a change to fix it on functional-test-repo after this is merged

Copy link
Member Author

Choose a reason for hiding this comment

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

Copy link
Contributor

@ylwu-amzn ylwu-amzn left a comment

Choose a reason for hiding this comment

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

Approved this PR to unblock 2.0 development. You can fix failed IT in separate PR.

@ananzh ananzh self-requested a review April 7, 2022 23:02
Copy link
Member

@ananzh ananzh left a comment

Choose a reason for hiding this comment

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

nice. thx Amit

@amitgalitz amitgalitz merged commit fc70a31 into opensearch-project:main Apr 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking dependencies Pull requests that update a dependency file v2.0.0 Version 2.0.0
Projects
None yet
4 participants