[GitHub] [airflow] potiuk edited a comment on pull request #10156: [GH-10147] Use conn_name_attr for SqliteHook connection

[GitBox <gi...@apache.org>]

potiuk edited a comment on pull request #10156:
URL: https://github.com/apache/airflow/pull/10156#issuecomment-668571347


   Thanks @coopergillan. I see good contributions coming from you :).
   
   One small friendly comment though :).
   
   I have a kind request to not use the [GH-NNNN] prefix in the subject of the PR. While I understand why you are doing it, we chose a little different approach in the community of Airflow. 
   
   Maybe it's not perfectly documented (but please take a look at PR/issue template), but one thing we want to have is tohave as much space in the subject of the PR/commit as possible rather than "clutter" it with - fairly artificial issue prefixes - [GH-NNNN]. We used to have JIRA issues this way and we hated it. It was quite a pain when we prepared release notes etc. It's quite a bit of noise especially that we also have PR numbers in commits after merge.
   
   Our approach is "one change per PR" and rather than Iissue#  we are using PR# to refer to each change. So rather than referring to particular issue being solved, we talk about particular PR merged (which might solve an issue) and in Release notes we are referring to this PR (not to the issue). It sounds counter-intuitive at first, but we think it's actually better as you can go directly to the code solving the problem by referring to PR rather than add an extra hop. The PR# number is automatically added by Github when you merge PR (with our squash+rebase) method. It adds (#NNN) added so if you refer to an issue in the subject of the commit you have two different numbers there:
   ```
   [G-XXXX] Some description (#YYYY)
   ```
   Where XXXX is the issue number and YYYY is the PR number.
   
   We chose PR# number as the "main" index then. If you do have an issue that you want to address with the PR you simply add 
   'Closes #NNNN'  anywhere in the detailed commit message (not the subject) and GitHub will close the issue automatically when it gets merged. This is what I've manually changed at the merge time in the PR of yours.
   
   One drawback of this solution (but one we accepted as a community) is that you have to split your PR into several PRs if they are referring to different things (particularly if they are solving two issues). It's a bit of a pain especially when you work on a chain of dependent changes (been there, done that) but we think long term it brings much better cherry-pickability and overall quality of the changes. 
   
   I hope it explains the reasons why we are doing 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