You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@uniffle.apache.org by "advancedxy (via GitHub)" <gi...@apache.org> on 2023/02/06 02:37:35 UTC
[GitHub] [incubator-uniffle] advancedxy opened a new issue, #551: [Improvement][CodingStyle] add autolink to issues in PR title and commit msg
advancedxy opened a new issue, #551:
URL: https://github.com/apache/incubator-uniffle/issues/551
### Code of Conduct
- [X] I agree to follow this project's [Code of Conduct](https://www.apache.org/foundation/policies/conduct)
### Search before asking
- [X] I have searched in the [issues](https://github.com/apache/incubator-uniffle/issues?q=is%3Aissue) and found no similar issues.
### What would you like to be improved?
It's common and considered best practice to be able to click links to corresponding issues/PRs/tickets in the commit msg. To achieve this features, Github's autolinked references and links[^1] are used.
For example, Spark uses this feature to autolink Jiras.
<img width="1251" alt="image" src="https://user-images.githubusercontent.com/807537/216869357-205d4cfb-7a37-47a4-beba-fef632399e57.png">
Since Uniffle are using Github Issues to track issues/request/improvements, etc. The issue is effective the same as the Jira ticket.
I'd like to propose an suggestion to improve the code context experience: add autolink to issues in PR title and commit msg.
[^1]: https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/autolinked-references-and-urls
### How should we improve?
1. Let's decide which form to include autolinks:
- [#xxx], the simple form, https://github.com/apache/incubator-uniffle/commit/cf9afab471a9595d8298ebd65c4d25c080ad4bbf
<img width="956" alt="image" src="https://user-images.githubusercontent.com/807537/216869878-3023c60d-b64c-46eb-a631-47c8dd6eb915.png">
- [ISSUE #xxx] prefix with issue
2. Enforce the pr title check. Every PR should have an issue linked.
However this may not applied to every case. There might be some exception rule to skip this check.
### Are you willing to submit PR?
- [X] Yes I am willing to submit a PR!
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on issue #551: [Improvement][CodingStyle] add autolink to issues in PR title and commit msg
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418439241
I would suggest omit issue id in the title, and just add it in the PR description.
```
### Why this change needed?
Fix #123
```
The PR title could be `[module][type] title`.
Modules:
* `coordinator`
* `server`
* `client`
* `operator`
* `*`: all
* etc
Types:
* `feature`: (new feature for the user, not a new feature for build script)
* `fix`: (bug fix for the user, not a fix to a build script)
* `docs`: (changes to the documentation)
* `style`: (formatting, missing semi colons, etc; no production code change)
* `refactor`: (refactoring production code, eg. renaming a variable)
* `test`: (adding missing tests, refactoring tests; no production code change)
* `chore`: (updating grunt tasks etc; no production code change)
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418471036
> The point to add issue id in the title is that we can links to issues and commits in Github's ecosystem. In most cases, only title is displayed for commits.
My point is, by just looking at the issue id, you don't get any information.
When looking at commit history, you got interested by the scope and type of the change first.
Then you start to looking for why it's needed, and find the related issue.
Just click the 3 dots, you could see it the description.
<img width="642" alt="image" src="https://user-images.githubusercontent.com/5821159/216879627-99b96924-c86e-4d3f-83ec-0a6f880d1836.png">
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][CodingStyle] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418430223
> Can we use the apache style like `[UNIFFLE-120][Test] Fix msg` ? And the `uniffle-120 `could be linked to github issue by https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/configuring-autolinks-to-reference-external-resources
No. The asf doesn't expose the configurations to external resource except JIRA which is already been closed for public registration.
I'd like to use `[ISSUE-#xxx]`, but it seems Github doesn't recognize that. You have to use `[ISSUE #XXX]` or `[Uniffle #xxx]` or just `[#xxx]`
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418476830
> > The point to add issue id in the title is that we can links to issues and commits in Github's ecosystem. In most cases, only title is displayed for commits.
>
> My point is, by just looking at the issue id, you don't get any information.
No, the auto-linked issues is clickable. You can jump to the issue page by simply looking at the title just as you can jump to the pr page.
<img width="1331" alt="image" src="https://user-images.githubusercontent.com/807537/216881209-5f0784d5-e1a9-4ec6-91de-2da3bcdd4e93.png">
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #551: [Improvement][CodingStyle] add autolink to issues in PR title and commit msg
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418423865
Can we use the apache style like `[UNIFFLE-120][Test] Fix msg` ? And the `uniffle-120 `could be linked to github issue by https://docs.github.com/en/repositories/managing-your-repositorys-settings-and-features/managing-repository-settings/configuring-autolinks-to-reference-external-resources
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] zuston commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "zuston (via GitHub)" <gi...@apache.org>.
zuston commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422309216
It's OK for me, thanks for your effort. @advancedxy
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422473337
How about combine issue ID and [semantic commit message][1] ?
```
[<issue>]<type>(<scope>): <subject>
```
For example:
```
[#123]feat(operator): support feature xxx
[minor]refactor: fix typo in variable name
```
[1]: https://gist.github.com/joshbuchea/6f47e86d2510bce28f8e7f42ae84c716
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on issue #551: [Improvement][CodingStyle] add autolink to issues in PR title and commit msg
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418428588
`UNIFFLE-120` may be confusing, because it's usually a Jira id.
Currently I would not recommend Jira because the self registration is limited. We wish to make new contributors easier to join the project.
For how to configure the autolink, see https://cwiki.apache.org/confluence/display/INFRA/Git+-+.asf.yaml+features
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][CodingStyle] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418418491
@kaijchen @zuston @jerqi @felixcheung WDYT?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422454531
Also FYI, I add an exception rule:
<img width="816" alt="image" src="https://user-images.githubusercontent.com/807537/217518921-ebea8591-8fec-4fda-816b-4a83d6e991f2.png">
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422302422
<img width="1206" alt="image" src="https://user-images.githubusercontent.com/807537/217491266-e19f71f6-e9e2-4798-8183-de8a907a6501.png">
@kaijchen @zuston does this demonstrate the benefits of autolinks?
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422423514
Seems the following format is preferred now?
```
[#123] feat: add xxx feature
[#124] fix: exception in xxx
```
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1418465944
> I would suggest omit issue id in the title, and just add it in the PR description.
>
> ```
> ### Why this change needed?
>
> Fix #123
> ```
>
The point to add issue id in the title is that we can links to issues and commits in Github's ecosystem. In most cases, only title is displayed for commits.
Another benefit to include issue id in PR title(included in commit title automatically) is the commit is also linked to the issue.
See https://github.com/apache/incubator-uniffle/issues/545 for example
<img width="993" alt="image" src="https://user-images.githubusercontent.com/807537/216879283-c4bdb453-46dd-422f-a5a3-75c95431671f.png">
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422303459
> kaijchen zuston jerqi felixcheung WDYT?
also cc @xianjingfeng which was pointed to a wrong one.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] kaijchen commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1425283306
Hi everyone, I've edited existing open PR titles (to show examples to new PR). How do you think?
<img width="732" alt="screenshot" src="https://user-images.githubusercontent.com/5821159/218024535-ac1a4c96-57a5-42cd-bd21-30148df4e5a5.png">
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] xianjingfeng commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "xianjingfeng (via GitHub)" <gi...@apache.org>.
xianjingfeng commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422389044
> > kaijchen zuston jerqi felixcheung WDYT?
>
> also cc @xianjingfeng which was pointed to a wrong one.
It's OK for me.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] advancedxy commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "advancedxy (via GitHub)" <gi...@apache.org>.
advancedxy commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422452999
> Seems the following format is preferred now?
>
> ```
> [#123] feat: add xxx feature
> [#124] fix: exception in xxx
> ```
It's preferred to use conventional commits. But I don't think all the committers are ready for this. Let's keep it as a personal preference for a while? If everyone agrees with conventional commits, we can also enforces this.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] jerqi commented on issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "jerqi (via GitHub)" <gi...@apache.org>.
jerqi commented on issue #551:
URL: https://github.com/apache/incubator-uniffle/issues/551#issuecomment-1422062201
It's ok for me.
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [incubator-uniffle] kaijchen closed issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
Posted by "kaijchen (via GitHub)" <gi...@apache.org>.
kaijchen closed issue #551: [Improvement][Style] add autolink to issues in PR title and commit msg
URL: https://github.com/apache/incubator-uniffle/issues/551
--
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.
To unsubscribe, e-mail: dev-unsubscribe@uniffle.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org