Skip to content

Revisiting our processes #74

Open
Open

Description

@deyaaeldeen

In an untyped world where bugs are looming over, we better revisit our engineering processes. My thoughts are to enforce successful completion of our test suite as a requirement before merging, perhaps throw in our benchmarks too? looking for performance improvements/bugs, and perhaps add code coverage checker. I also think we should protect master, we can discuss if this is also required for admins. The end goal of this issue is to come up with new checklist of requirements on PRs.

Activity

  1. akuhlens commented on Feb 7, 2019

    @akuhlens
    Collaborator
    • I think a lot of our focus should be on having obvious unit testing in any new / modified development.
      • Any modification of a function needs to add the unit tests
      • Any removal or modification of a previous unit test should be examined closely (by a reviewer)
    • The default test requirements for a PR should be to run the integration tests and all the unit test.
      • It would be nice if the manually running the benchmarks wasn't needed
      • And is probably worth saving time on most small pull requests by not requiring the benchmarks
        to finish before accepting a PR. (We will usually be able to reset a pull request)
    • I think there should be some sort of standardization of reviews based on the scale of the change.
      • Admins on small pull requests don't need review (but still must PR) and only require an informal
        review for non-admin.
      • Medium PRs can be quickly reviewed (based on tests, comments, and style)
      • Large PRs must be understood as whole
      • The difference between small, medium, and large should not just be code size
    • Do we have a style guide yet?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions