Update approve-to-run-ci.yml

[duckling69]

Description

This PR fixes #474

Notes for Reviewers

Signed commits

• Yes, I signed my commits.

[welcome]

Yay, your first pull request! 👍 A contributor will be by to give feedback soon. In the meantime, please review the Layer5 Community Welcome Guide and sure to join the community Slack.
Be sure to double-check that you have signed your commits. Here are instructions for making signing an implicit activity while peforming a commit.

[codecov]

Codecov Report

Patch and project coverage have no change.

Comparison is base (6383cbf) 33.86% compared to head (e4bb9f2) 33.86%.

❗ Current head e4bb9f2 differs from pull request most recent head 41ac06d. Consider uploading reports for the commit 41ac06d to get more accurate results

Additional details and impacted files

@@           Coverage Diff           @@
##           master     #477   +/-   ##
=======================================
  Coverage   33.86%   33.86%           
=======================================
  Files           6        6           
  Lines         375      375           
=======================================
  Hits          127      127           
  Misses        241      241           
  Partials        7        7           

Flag	Coverage Δ
unittests	33.86% <ø> (ø)

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

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[Aisuko]

LGTM

[leecalcote]

If we're going to move forward with commands like this, all repos should support this behavior universally and consistently.

Command should be prepended with / so as to avoid clashing with commonly used comments like the one that @Aisuko gave above.

Please raise this PR and this question in the discuss.layer5.io forum as a proposal for change across all repos.

[leecalcote]

Also, I generally agree with @Aisuko in terms of this PR: LGTM. Let's review as a broader proposal, though.

[Aisuko]

If we're going to move forward with commands like this, all repos should support this behavior universally and consistently.

Command should be prepended with / so as to avoid clashing with commonly used comments like the one that @Aisuko gave above.

Please raise this PR and this question in the discuss.layer5.io forum as a proposal for change across all repos.

Will do.

[Aisuko]

Hi @duckling69 , May you help check the why the Build CI was not triggered? I believe it is not too much useful. And we can change to the simple one which is only adding need key word for the Build job.

[duckling69]

Hi @duckling69 , May you help check the why the Build CI was not triggered? I believe it is not too much useful. And we can change to the simple one which is only adding need key word for the Build job.

Will have a look into it

[duckling69]

If we're going to move forward with commands like this, all repos should support this behavior universally and consistently.

Command should be prepended with / so as to avoid clashing with commonly used comments like the one that @Aisuko gave above.

Please raise this PR and this question in the discuss.layer5.io forum as a proposal for change across all repos.

Just a suggestion maybe we can use something like zappr to manage these triggers I had a look into it while solving the issue

[Aisuko]

Hi @duckling69 , May you help check the why the Build CI was not triggered? I believe it is not too much useful. And we can change to the simple one which is only adding need key word for the Build job.

Will have a look into it

Hi @duckling69
If it is not working so well, let's add a need: [job1, job2] condition for the build job. And we can get a quick merge.

[duckling69]

Hi @duckling69 , May you help check the why the Build CI was not triggered? I believe it is not too much useful. And we can change to the simple one which is only adding need key word for the Build job.

Will have a look into it

Hi @duckling69 If it is not working so well, let's add a need: [job1, job2] condition for the build job. And we can get a quick merge.

Sure done

[welcome]

        Thank you for contributing to the Layer5 community! 🎉 \ \ \ \         ⭐ Please leave a star on the project. 😄