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

Add sentence about test coverage to CONTRIBUTING.md #755

Closed
simonsan opened this issue Jul 17, 2023 · 3 comments · Fixed by #762 or #820
Closed

Add sentence about test coverage to CONTRIBUTING.md #755

simonsan opened this issue Jul 17, 2023 · 3 comments · Fixed by #762 or #820
Labels
A-docs Area: Improvements or additions to documentation A-meta Area: Project wide

Comments

@simonsan
Copy link
Contributor

simonsan commented Jul 17, 2023

We should add a CONTRIBUTING.md containing a few guidelines and things people should know before contributing:

A - Area
C - Category
D - Diagnostic
E - Call for participation
F - Feature
I - Issue e.g. I-crash
M - Meta
O - Operating systems
P - priorities e.g. P-{low, medium, high, critical}
PG - Project Group
perf - Performance
S - Status e.g. S-{blocked, experimental, inactive}
T - Team relevancy
WG - Working group
  • guideline should state clearly that code that is changed and PRed needs to be tested (so the old code needs to be covered, and the test needs to be passing for the new code as well)

    • test coverage currently is low but we need a higher test coverage to refactor stuff, so this guideline seems reasonable
    • like this we raise continuously the test coverage and make sure we don't pull in logical bugs, while we can still accept PRs
  • workflow for PRs

    • e.g. not force pushing, so we can keep review comments
  • example CONTRIBUTING.md: https://github.com/SFTtech/openage/blob/master/doc/contributing.md

@simonsan simonsan added A-docs Area: Improvements or additions to documentation A-meta Area: Project wide labels Jul 17, 2023
@simonsan simonsan added this to the Road to v1 milestone Aug 8, 2023
@simonsan simonsan reopened this Aug 10, 2023
@simonsan simonsan changed the title Add CONTRIBUTING.md Add sentence about test coverage to CONTRIBUTING.md Aug 10, 2023
@simonsan
Copy link
Contributor Author

guideline should state clearly that code that is changed and PRed needs to be tested (so the old code needs to be covered, and the test needs to be passing for the new code as well)

  • test coverage currently is low but we need a higher test coverage to refactor stuff, so this guideline seems reasonable
  • like this we raise continuously the test coverage and make sure we don't pull in logical bugs, while we can still accept PRs

@aawsome What do you think, should we add this as well? So people think of their contributions as something that they autonomously add testing to? Thinking of it in the way: "If I implement this I want to know if it's working as expected!" Like a self expectation kinda thing?

@aawsome
Copy link
Member

aawsome commented Aug 10, 2023

I would add that we expect PRs to contain tests for the new code and that we welcome PRs which also increase the general test coverage. Maybe add that PRs with test have a much higher probability of getting merged (fast).

@simonsan
Copy link
Contributor Author

I would add that we expect PRs to contain tests for the new code and that we welcome PRs which also increase the general test coverage. Maybe add that PRs with test have a much higher probability of getting merged (fast).

Yes, that sounds good! Will add!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-docs Area: Improvements or additions to documentation A-meta Area: Project wide
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants