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

Remove scripts from notifications repo due to naming of the entries in manifest file #404

Merged

Conversation

peterzhuamazon
Copy link
Member

@peterzhuamazon peterzhuamazon commented Apr 12, 2022

Signed-off-by: Peter Zhu [email protected]

Description

Remove scripts from notifications repo due to naming of the entries in manifest file

Issues Resolved

#379

Check List

  • New functionality includes testing.
    • All tests pass, including unit test, integration test and doctest
  • New functionality has been documented.
    • New functionality has javadoc added
    • New functionality has user manual doc added
  • 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.

@peterzhuamazon
Copy link
Member Author

This will go in parallel with opensearch-project/opensearch-build#1957

@codecov-commenter
Copy link

codecov-commenter commented Apr 13, 2022

Codecov Report

Merging #404 (543d6ca) into main (731ee82) will decrease coverage by 10.00%.
The diff coverage is n/a.

@@              Coverage Diff              @@
##               main     #404       +/-   ##
=============================================
- Coverage     70.38%   60.37%   -10.01%     
  Complexity       81       81               
=============================================
  Files           123       72       -51     
  Lines          3849     2370     -1479     
  Branches        612      260      -352     
=============================================
- Hits           2709     1431     -1278     
+ Misses          961      763      -198     
+ Partials        179      176        -3     
Flag Coverage Δ
dashboards-notifications ?
opensearch-notifications 60.37% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...tions/public/pages/Emails/CreateRecipientGroup.tsx
dashboards-notifications/public/utils/helpers.ts
.../pages/CreateChannel/components/WebhookHeaders.tsx
...ages/CreateChannel/components/ChannelNamePanel.tsx
.../Emails/components/tables/SendersTableControls.tsx
...Channel/components/modals/CreateSESSenderModal.tsx
...ateChannel/components/modals/CreateSenderModal.tsx
...notifications/public/pages/Emails/EmailSenders.tsx
...Channels/components/details/ChannelDetailItems.tsx
...CreateChannel/components/CustomWebhookSettings.tsx
... and 41 more

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 731ee82...543d6ca. Read the comment docs.

@peterzhuamazon
Copy link
Member Author

Make change to remove the scripts from notifications repo due to the manifest name field cannot matching the repo name of notifications due to notifications-core and notifications-notifications subfolder structure.

Signed-off-by: Peter Zhu <[email protected]>
@peterzhuamazon peterzhuamazon changed the title Add new build scripts for subfolder structure Remove scripts from notifications repo due to naming of the entries in manifest file Apr 13, 2022
@rishabhmaurya
Copy link
Contributor

copying artifacts part can go in a gradle task which can be defined in the parent project of notification plugin and would keep things cleaner.
Approving it now and this can be cleaned up later!

@peterzhuamazon peterzhuamazon merged commit 5421f98 into opensearch-project:main Apr 13, 2022
@peterzhuamazon peterzhuamazon deleted the opensearch-2.0.0-builds branch April 13, 2022 01:58
@dblock
Copy link
Member

dblock commented Apr 13, 2022

@rishabhmaurya Where can I read more about how notifications-core came to be, and why isn't it in its own repo? There are multiple hacks that were required to make it part of the distribution, starting with opensearch-project/opensearch-build#1959, and I suspect there will be more interesting hacks downstream when we get to publishing these to maven, or try to implement some other feature.

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

Successfully merging this pull request may close these issues.

8 participants