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

suggest the "Requirements before merging" section be removed from PR template, until we better support these process elements #913

Open
edwardhartnett opened this issue Jan 21, 2025 · 0 comments

Comments

@edwardhartnett
Copy link
Collaborator

In the PR submission form we have:

# Requirements before merging
- [ ] All new code in this PR is tested by at least one unit test
- [ ] All new code in this PR includes Doxygen documentation
- [ ] All new code in this PR does not add new compilation warnings (check CI output)

It's too soon to list these in the PR form. Users have no framework of unit tests to use, nor any example. The doxygen in the code is inconsistent and fragmentary.

The warnings is a good aspiration, but until we have a reasonably warning-free build it can be hard to tell if you have added any warnings.

Project leadership need to provide more support for these process elements, before throwing them to the programmers. We should not expect our programmers to be as heroic as that! ;-)

I suggest that these be removed from the template, and re-added incrementally, when we can provide better support to programmers for these activities. For example once we get the doxygen warning-free build working, and we've provided some doxygen training to the programmers, then we can add the requirement that all code have complete and correct doxygen documentation.

It will be great when we can provide programmers with a well-documented code base, and expect them to maintain and extend that high standard of documentation. But we have some catch-up work to do first. Once the code base is fully and consistently doxygenated, and a CI build confirms that new work conforms, it will be a lot easier for the programmer to know what to do.

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

No branches or pull requests

1 participant