-
Notifications
You must be signed in to change notification settings - Fork 120
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
[AlgoCollector] Pull request requirement #1460
Comments
I think I can take this issue |
@ledongthuc I don't think Github API provided a list of changed files when we get PR list. To have this we need another API call per PR. I don't like this approach. Instead, we have PR's body information that might contain information we needed:
For example, with a PR like above, we can make sure a PR is for algorithm submit by checking for What do you think? |
I think we should create a convention for the Algorithm Weekly Pull request title such as [Algorithm] or [Algorithm Weekly] |
That’s a good idea. What prefix should we use? [Algorithm] or [Algorithm Week]?
cc @ledongthuc
Sent from ProtonMail for iOS
…On Sun, Aug 8, 2021 at 7:13 PM, Ro Ngoc Vo ***@***.***> wrote:
I think we should create a convention for the Algorithm Weekly Pull request title such as [Algorithm] or [Algorithm Weekly]
i.e "[Algorithm Weekly] 2021 - SS2 - Week4 - khoa" or something like that. By using it, the PR checking will be more consistent and reliable.
—
You are receiving this because you commented.
Reply to this email directly, [view it on GitHub](#1460 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AVEGOHIYTAVABQGYR5L362LT342UNANCNFSM5BYGSU3Q).
Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email).
|
@ZephyrOneD @ngocro208, In case Github API doesn't support changed files in API listing PRs, maybe detect patterns is a good solution. I think PR's title prefix is better than body content parsing, the But it requires everyone must follow the title convention. Another option if we want to detect files is using a Github action that use to add labels to Pull Request bases on their file changes. Then AlgoCollector will read the PR label to detect. https://github.com/actions/labeler If you need to support to add the github action to test, I can do it. |
Personally I think most people will easier to forgot to add a Label when they create a PR, and it's easier to do that with Title.
Let's go with the Title prefix, [Algorithm Week] or [Algorithm] should both valid.
Sent with [ProtonMail](https://protonmail.com/) Secure Email.
‐‐‐‐‐‐‐ Original Message ‐‐‐‐‐‐‐
…On Monday, August 9th, 2021 at 1:10 AM, Thuc Le ***@***.***> wrote:
***@***.***(https://github.com/zephyroned) ***@***.***(https://github.com/ngocro208), In case Github API doesn't support changed files in API listing PRs, maybe detect patterns is a good solution. I think PR's title prefix is better than body content parsing, the [Algorithm].
But it requires everyone must follow the title convention.
Another option if we want to detect files is using a Github action that use to add labels to Pull Request bases on their file changes. Then AlgoCollector will read the PR label to detect.
https://github.com/actions/labeler
If you need to support to add the github action to test, I can do it.
—
You are receiving this because you were mentioned.
Reply to this email directly, [view it on GitHub](#1460 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AVEGOHP6MZN4JDNXOMJEVW3T36EPRANCNFSM5BYGSU3Q).
Triage notifications on the go with GitHub Mobile for [iOS](https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675) or [Android](https://play.google.com/store/apps/details?id=com.github.android&utm_campaign=notification-email).
|
Ok, finally, I suggest using both title prefix [Algorithm Week] or [Algorithm] or has the label "algo" (e.g. https://github.com/ruby-vietnam/hardcore-rule/pulls?q=is%3Apr+is%3Aclosed+label%3Aalgo) |
hi @huytd, I added a Github action that automatically adds the label |
Hi @ledongthuc, the labeler does not work for forks repo . I think we could apply the PR title prefix for now. |
Source code: https://github.com/ruby-vietnam/hardcore-rule/tree/master/algorithms/collector
Background
Problems
Proposal
The text was updated successfully, but these errors were encountered: