Skip to content
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

[AIRFLOW-6394] Simplify github PR template #6955

Merged

Conversation

potiuk
Copy link
Member

@potiuk potiuk commented Dec 29, 2019

Our Github template is not really useful for most people. Almost no-one is
marking the tick-marks when submitting the PR and not many people update the
link to the AIRFLOW ticket. I don't think it serves the purpose of teaching
people as it is too long to be actually read and acted upon

We should simplify it.

And later we should add probot checks to automate most of the stuff that we
wanted to enforce with the template

In case of fundamental code change, Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in UPDATING.md.
Read the Pull Request Guidelines for more information.

Copy link
Contributor

@BasPH BasPH left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea, the current template isn't very useful.

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
@potiuk potiuk force-pushed the AIRFLOW-6394-simplify-github-template branch 4 times, most recently from 4144bc4 to e5b90f6 Compare December 29, 2019 20:33
@potiuk
Copy link
Member Author

potiuk commented Dec 29, 2019

I removed some more unneeded words :)

Copy link
Member

@turbaszek turbaszek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the simplicity.

@mik-laj
Copy link
Member

mik-laj commented Dec 29, 2019

I think it is worth keeping the requirement to add a link to JIRA in the description. Now it is not very clear because it is in the middle of a sentence.

@turbaszek
Copy link
Member

I think it is worth keeping the requirement to add a link to JIRA in the description. Now it is not very clear because it is in the middle of a sentence.

I agree and moreover we have to adjust XXXX two times?

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
@potiuk potiuk force-pushed the AIRFLOW-6394-simplify-github-template branch from 3e736fe to f644e99 Compare December 30, 2019 07:31
@potiuk
Copy link
Member Author

potiuk commented Dec 30, 2019

I updated the template after the comments. Please re-review :). You can see how such PR will look like above - I updated PR description according to the updated template.

I think it's much cleaner and actually actionable and more readable and useful (comparing to the old one). Let me know if you are happy with it !

@potiuk potiuk force-pushed the AIRFLOW-6394-simplify-github-template branch 3 times, most recently from 706ae4a to c818275 Compare December 30, 2019 07:37
- All the public functions and the classes in the PR contain docstrings that explain what it does
- If you implement backwards incompatible changes, please leave a note in the [Updating.md](https://github.com/apache/airflow/blob/master/UPDATING.md) so we can assign it to a appropriate release
- [ ] Description above provides context of the change
- [ ] Commit message contains [\[AIRFLOW-XXXX\]](https://issues.apache.org/jira/browse/AIRFLOW-XXXX) or `[AIRFLOW-XXXX]` for document-only changes
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I right that when opening a PR I have to input JIRA number two times? I think it's redundant

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nuclearpinguin. I tried to simplify it indeed but do not know how. Any idea how to make it not redundant before we employ probot ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's super useful to have the link here so that we can quickly move to the JIRA issue. We can do it with probot likely but now some manual entry is needed I am afraid :(

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine, generally, you just copy/paste it. Just 1 more paste :)

@baolsen
Copy link
Contributor

baolsen commented Dec 30, 2019

Suggestion, as a new contributor I was at first not aware of the process for requesting reviews. I had read the contribution guidelines but it was not 100% clear to me how code review worked.

I've since thought that something could be mentioned on this checklist, e.g. -
[X] I will engage with the code review process as explained in the contribution guidelines

(IMHO I also feel that the contribution guidelines should have a separate section for code review process, so it is clearer and easy to find for newbies)

@potiuk
Copy link
Member Author

potiuk commented Dec 30, 2019

Suggestion, as a new contributor I was at first not aware of the process for requesting reviews. I had read the contribution guidelines but it was not 100% clear to me how code review worked.

I've since thought that something could be mentioned on this checklist, e.g. -
[X] I will engage with the code review process as explained in the contribution guidelines

(IMHO I also feel that the contribution guidelines should have a separate section for code review process, so it is clearer and easy to find for newbies)

Would Contribution Workflow Example work @baolsen? We've added it recently as part of GSOD changes.

Our Github template is not really useful for most people. Almost no-one is
marking the tick-marks when submitting the PR and not many people update the
link to the AIRFLOW ticket. I don't think it serves the purpose of teaching
people as it is too long to be actually read and acted upon

We should simplify it.

And later we should add probot checks to automate most of the stuff that we
wanted to enforce with the template
@potiuk potiuk force-pushed the AIRFLOW-6394-simplify-github-template branch from c818275 to b00824b Compare December 30, 2019 12:24
@kaxil kaxil merged commit 44e8ebd into apache:master Dec 30, 2019
potiuk added a commit that referenced this pull request Dec 30, 2019
potiuk added a commit that referenced this pull request Jan 21, 2020
galuszkak pushed a commit to FlyrInc/apache-airflow that referenced this pull request Mar 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants