You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@helix.apache.org by GitBox <gi...@apache.org> on 2020/08/03 00:49:22 UTC
[GitHub] [helix] jiajunwang opened a new pull request #1209: Add Github actions to enable auto CI tests.
jiajunwang opened a new pull request #1209:
URL: https://github.com/apache/helix/pull/1209
### Issues
- [X] My PR addresses the following Helix issues and references them in the PR description:
#939
### Description
- [X] Here are some details about my PR, including screenshots of any UI changes:
1. On any code merge to master, run the full test.
2. For PR creating, or updating, run the minimum required tests based on the changed files.
### Tests
- [ ] The following tests are written for this issue:
(List the names of added unit/integration tests)
- [ ] The following is the result of the "mvn test" command on the appropriate module:
(Copy & paste the result of "mvn test")
### Commits
- [X] My commits all reference appropriate Apache Helix GitHub issues in their subject lines. In addition, my commits follow the guidelines from "[How to write a good git commit message](http://chris.beams.io/posts/git-commit/)":
1. Subject is separated from body by a blank line
1. Subject is limited to 50 characters (not including Jira issue reference)
1. Subject does not end with a period
1. Subject uses the imperative mood ("add", not "adding")
1. Body wraps at 72 characters
1. Body explains "what" and "why", not "how"
### Documentation (Optional)
- [ ] In case of new functionality, my PR adds documentation in the following wiki page:
(Link the GitHub wiki you added)
### Code Quality
- [X] My diff has been formatted using helix-style.xml
(helix-style-intellij.xml if IntelliJ IDE is used)
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] kaisun2000 commented on pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
kaisun2000 commented on pull request #1209:
URL: https://github.com/apache/helix/pull/1209#issuecomment-668172896
This looks great. Two questions:
1/ will the history of CI test be available somewhere?
2/ is there a link about how these CI config works?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
narendly commented on pull request #1209:
URL: https://github.com/apache/helix/pull/1209#issuecomment-667820076
@jiajunwang
Please follow the pull request description guidelines. It's recommended that we use the following keywords:
close
closes
closed
fix
fixes
fixed
resolve
resolves
resolved
when referring to the issue number. This is clearly outlined in our guidelines so that when the PR gets closed, the issue gets linked and closed as well. See https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on pull request #1209:
URL: https://github.com/apache/helix/pull/1209#issuecomment-668159602
> @jiajunwang
>
> Please follow the pull request description guidelines. It's recommended that we use the following keywords:
>
> close
> closes
> closed
> fix
> fixes
> fixed
> resolve
> resolves
> resolved
>
> when referring to the issue number. This is clearly outlined in our guidelines so that when the PR gets closed, the issue gets linked and closed as well. See https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue
I think you just modified the guideline. But I didn't get any notification or discussion invitation. So let's have a talk about it. Since I read through the provided link, the keywords are a tool so that you can auto-link the issue with some assumptions, which I'm not sure if they help. Although I do agree to use the linked Issue feature.
1.
> Only **manually** linked pull requests can be manually unlinked. To unlink an issue that you linked using a keyword, you must edit the pull request description to remove the keyword.
Editing pull request description to unlink seems a bad idea. It might be easier just to manually link.
2.
> You can also use closing keywords in a commit message. The issue will be closed when you merge the commit into the default branch, but the pull request that contains the commit will not be listed as a linked pull request.
"default branch" means master, I think. So if we are using a dev branch, then the ticket will not be closed. Moreover, this implicit close might close some issues prematurely. So I won't recommend it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on pull request #1209:
URL: https://github.com/apache/helix/pull/1209#issuecomment-668199754
This PR is ready to be merged, approved by @kaisun2000
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] narendly commented on a change in pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
narendly commented on a change in pull request #1209:
URL: https://github.com/apache/helix/pull/1209#discussion_r464207491
##########
File path: .github/workflows/Helix-PR-CI.yml
##########
@@ -0,0 +1,46 @@
+# Run the mininum required tests for pull requests.
Review comment:
Will this block PR merges upon test failure? If so, then we should reconsider adding this because it may not be reasonable to assume all tests will pass consistently.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang commented on a change in pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
jiajunwang commented on a change in pull request #1209:
URL: https://github.com/apache/helix/pull/1209#discussion_r464249393
##########
File path: .github/workflows/Helix-PR-CI.yml
##########
@@ -0,0 +1,46 @@
+# Run the mininum required tests for pull requests.
Review comment:
I don't think it will block anything. Since I can still merge code to my forked repo. I think this just provides additional support.
On the other hand, if it is not the case now, then we shall delay this PR. But we will need this eventually.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang edited a comment on pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
jiajunwang edited a comment on pull request #1209:
URL: https://github.com/apache/helix/pull/1209#issuecomment-668159602
> @jiajunwang
>
> Please follow the pull request description guidelines. It's recommended that we use the following keywords:
>
> close
> closes
> closed
> fix
> fixes
> fixed
> resolve
> resolves
> resolved
>
> when referring to the issue number. This is clearly outlined in our guidelines so that when the PR gets closed, the issue gets linked and closed as well. See https://docs.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue
I think you just modified the guideline. But I didn't get any notification or discussion invitation. So let's have a talk about it. Since I read through the provided link, the keywords are a tool so that you can auto-link the issue with some assumptions, which I'm not sure if they help. Although I do agree to use the linked Issue feature.
1.
> Only **manually** linked pull requests can be manually unlinked. To unlink an issue that you linked using a keyword, you must edit the pull request description to remove the keyword.
Editing pull request description to unlink seems a bad idea. It might be easier just to manually link.
2.
> You can also use closing keywords in a commit message. The issue will be closed when you merge the commit into the default branch, but the pull request that contains the commit will not be listed as a linked pull request.
"default branch" means master, I think. So if we are using a dev branch, then the ticket will not be closed. Moreover, this implicit close might close some issues prematurely. So I won't recommend it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] jiajunwang merged pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
jiajunwang merged pull request #1209:
URL: https://github.com/apache/helix/pull/1209
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org
[GitHub] [helix] kaisun2000 edited a comment on pull request #1209: Add Github actions to enable auto CI tests.
Posted by GitBox <gi...@apache.org>.
kaisun2000 edited a comment on pull request #1209:
URL: https://github.com/apache/helix/pull/1209#issuecomment-668172896
This looks great. Two questions:
1/ will the history of CI test be available somewhere?
Yes, after I check this in, you will see it under "action" tag in the project page.
2/ is there a link about how these CI config works?
Please google github actions for CI
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org