Skip to content

Refactor CONTRIBUTING.md#5048

Open
moffatsadeghi wants to merge 1 commit intomasterfrom
moffatsadeghi-contributingupdates
Open

Refactor CONTRIBUTING.md#5048
moffatsadeghi wants to merge 1 commit intomasterfrom
moffatsadeghi-contributingupdates

Conversation

@moffatsadeghi
Copy link
Collaborator

Updated formatting and wording for clarity in the CONTRIBUTING.md file.

Expanded the section on testing and pull request review process

Updated formatting and wording for clarity in the CONTRIBUTING.md file.

Expanded the section on testing and pull request review process
@johrstrom
Copy link
Contributor

I wonder if we can relax the language on requirements. Like yes we'd like tests, but I don't know if we should be enforcing the need for tests.

Or is it rather that we start enforcing that on each other? I mean we often make changes without tests ourselves, so we'd have to be a lot more strict with staff/maintainer pull requests too if we start enforcing the same on external contributors.

@Bubballoo3
Copy link
Contributor

To record here what was said in meetings, my feeling is that it is OK to require tests from external contributors but not from maintainers because while we can defer testing while remaining responsible for it, we cannot rely on future testing from an external contributor, nor hold them responsible for it in any way.

So I'd be in favor of leaving the language as is, and if for any reason testing does not apply or is especially difficult, it becomes a discussion with maintainers during the review process.

@moffatsadeghi
Copy link
Collaborator Author

Thanks for chiming in! If we are all good, then I think this can be merged unless I am missing something I need to fix in your feedback

Copy link
Contributor

@johrstrom johrstrom left a comment

Choose a reason for hiding this comment

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

No, we can merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Awaiting Review

Development

Successfully merging this pull request may close these issues.

3 participants