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

add convention for plugin location (relates to #13879) #233

Merged

Conversation

AmiStrn
Copy link
Contributor

@AmiStrn AmiStrn commented Jun 4, 2024

Description

adding convention for where plugin code should be

Issues Resolved

opensearch-project/OpenSearch#13879

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.

Signed-off-by: Amitai Stern <123amitai@gmail.com>
Copy link
Member

@andrross andrross left a comment

Choose a reason for hiding this comment

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

A few wording nitpicks, but overall I like it

CONVENTIONS.md Outdated Show resolved Hide resolved
CONVENTIONS.md Outdated Show resolved Hide resolved
CONVENTIONS.md Outdated Show resolved Hide resolved
AmiStrn and others added 3 commits June 4, 2024 16:13
Co-authored-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: amitai stern <123amitai@gmail.com>
Co-authored-by: Andrew Ross <andrross@amazon.com>
Signed-off-by: amitai stern <123amitai@gmail.com>
Signed-off-by: Amitai Stern <123amitai@gmail.com>
@AmiStrn
Copy link
Contributor Author

AmiStrn commented Jun 4, 2024

A few wording nitpicks, but overall I like it

Thanks @andrross :)
accepted the changes

@AmiStrn AmiStrn requested a review from andrross June 4, 2024 23:19
@AmiStrn
Copy link
Contributor Author

AmiStrn commented Jun 6, 2024

awaiting a second reviewer: @dblock @VachaShah @andrross @saratvemulapalli

@AmiStrn
Copy link
Contributor Author

AmiStrn commented Jun 6, 2024

oh sorry @andrross i didn't see you are not on the maintainer list for this repo. thanks for the review though.
@dblock
@VachaShah
@saratvemulapalli
@ohltyler

@andrross
Copy link
Member

andrross commented Jun 6, 2024

oh sorry @andrross i didn't see you are not on the maintainer list for this repo

And here I thought you just really valued my opinion :)

@AmiStrn
Copy link
Contributor Author

AmiStrn commented Jun 6, 2024

oh sorry @andrross i didn't see you are not on the maintainer list for this repo

And here I thought you just really valued my opinion :)

lol, that is NOT how I wished to come across. sarcasm aside (some have told me I am incapable of sarcasm), your opinion is very much valued!

dblock
dblock previously approved these changes Jun 7, 2024
Copy link
Member

@dblock dblock left a comment

Choose a reason for hiding this comment

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

I made some nitpicky comments, feel free to use/ignore/resolve.

CONVENTIONS.md Outdated Show resolved Hide resolved
CONVENTIONS.md Outdated Show resolved Hide resolved
CONVENTIONS.md Outdated Show resolved Hide resolved
CONVENTIONS.md Outdated Show resolved Hide resolved
Signed-off-by: Amitai Stern <123amitai@gmail.com>
@AmiStrn AmiStrn dismissed stale reviews from dblock and saratvemulapalli via 254ac97 June 10, 2024 19:57
@AmiStrn AmiStrn requested a review from dblock June 10, 2024 19:57
@AmiStrn
Copy link
Contributor Author

AmiStrn commented Jun 11, 2024

need one other approval for this one 🙏

@dblock
Copy link
Member

dblock commented Jun 12, 2024

I pinged @VachaShah for #229.

@dblock dblock merged commit 789b855 into opensearch-project:main Jun 17, 2024
4 checks passed
@AmiStrn AmiStrn deleted the convention-for-plugin-location branch June 19, 2024 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants