-
Notifications
You must be signed in to change notification settings - Fork 21
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
adding github action to test plugin installation via homebrew #158
adding github action to test plugin installation via homebrew #158
Conversation
Signed-off-by: Sanskar Bhushan <[email protected]>
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: sbdtu5498 The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Welcome @sbdtu5498! It looks like this is your first PR to knative-extensions/homebrew-kn-plugins 🎉 |
There was a problem hiding this 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! This is very much appreciated. I've added a couple of suggestions, but I looks good in general.
on: | ||
push: | ||
branches: | ||
- main |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest a common trigger combination that we use in other Knative GH actions.
on:
push:
branches: [ 'main', 'release-*' ]
pull_request:
branches: [ 'main', 'release-*' ]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I will change it accordingly.
brew install admin | ||
brew install func | ||
brew install quickstart | ||
brew install source-kafka | ||
brew install source-kamelet | ||
brew install event |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you try to extract the plugins to matrix and iterate over the items? That would be helpful to reuse the steps for MacOS, hence my comment above.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it would be better to keep the code dry.
|
||
jobs: | ||
check_plugins: | ||
runs-on: ubuntu-latest |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MacOS variant would be nice as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will add a MacOS variant as well.
@sbdtu5498 Hi there, a gentle ping! I'm wondering if you are still looking at this PR. |
@dsimansk I will work on this soon. I was a bit busy with some other task in hand. I will try to get it merged this week positively. |
Sure, no worries. Thanks for the update! It isn't super urgent to have it done this week, take your time. I wanted to confirm it's still on your radar. |
Super seeded by: #183 /close |
@dsimansk: Closed this PR. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
adding github action to test plugin installation via homebrew
/kind enhancement
Fixes #66
cc: @csantanapr
Release Note
Docs