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

[INFRA] Update PR template to add test and user change question #5486

Merged
merged 4 commits into from Sep 18, 2023

Conversation

Hisoka-X
Copy link
Member

@Hisoka-X Hisoka-X commented Sep 14, 2023

Purpose of this pull request

Update PR template to make sure contributer to check the change with test covered and it is or not change user-facing change. It will look like this PR msg.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Unnecessary.

Check list

@Hisoka-X Hisoka-X added the CI&CD label Sep 14, 2023
@EricJoy2048
Copy link
Member

Good Job, Can you add some well done tests or document links for contributors to refer to directly?

@Hisoka-X
Copy link
Member Author

Good Job, Can you add some well done tests or document links for contributors to refer to directly?

Code changes involve many different modules, and providing test cases of a certain module may mislead the contributer. The document too, and the all doc link already in check list.

@EricJoy2048
Copy link
Member

Good Job, Can you add some well done tests or document links for contributors to refer to directly?

Code changes involve many different modules, and providing test cases of a certain module may mislead the contributer. The document too, and the all doc link already in check list.

I found most of the PRs don't check the data type and value in Sink Connector e2e. And many new connector document did not use our latest document specification, but instead used the old one.

@Hisoka-X
Copy link
Member Author

Good Job, Can you add some well done tests or document links for contributors to refer to directly?

Code changes involve many different modules, and providing test cases of a certain module may mislead the contributer. The document too, and the all doc link already in check list.

I found most of the PRs don't check the data type and value in Sink Connector e2e. And many new connector document did not use our latest document specification, but instead used the old one.

Sounds reasonable, I add some comment for this.

Copy link
Member

@ruanwenjun ruanwenjun left a comment

Choose a reason for hiding this comment

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

LGTM

@ruanwenjun ruanwenjun added the Chore Chores about the project, like code cleaning up, typos, upgrading dependencies, etc. label Sep 15, 2023
@liugddx liugddx merged commit 31b375d into apache:dev Sep 18, 2023
4 checks passed
@Hisoka-X Hisoka-X deleted the update-pr-template branch September 19, 2023 09:44
gnehil pushed a commit to gnehil/seatunnel that referenced this pull request Oct 12, 2023
…he#5486)

* [INFRA] Update PR template to add test and user change question

* [INFRA] Update PR template to add test and user change question

* update
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved Chore Chores about the project, like code cleaning up, typos, upgrading dependencies, etc. CI&CD reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants