Consider implementing pre-commit hooks to ensure that tests are passing

[robyngit]

@ianguerin brought up a recent issue where an amendment to his PR resulted in failing tests that initially went unnoticed, and suggested adding a pre-commit hook to to ensure all unit tests pass before committing. Ian shared a simple implementation using husky.

Here are some notes from our discussion thus far:

• The primary benefit is preventing code that breaks existing and new unit tests from being committed.
• A secondary benefit is reducing the burden on PR reviewers
• strict pre-commit hooks might be a barrier for new or less experienced contributors, we may explore less restrictive alternatives that still encourage test-aware development, such as:
  • A custom message to inform the user about test failures without blocking the commit
  • enabling compliance checks on PRs instead of pre-commit hooks, allowing branch commits for collaborative work on WIP code while ensuring test passing before merging into develop.
• MetacatUI already has branch protection preventing merging without passing CI tests.
• new contributors might not be aware of where to find test status information on GitHub.
• There are potential limitations of pre-commit hooks for developers who prefer to commit incremental changes that may not pass tests immediately.
• To the contrary, it's possible to use the --no-verify flag for work-in-progress or experimental commits.

The team seeks to further discuss and weigh the pros and cons of implementing pre-commit hooks in the MetacatUI repository, considering factors like developer workflow, contribution accessibility, and the balance between strictness and flexibility. The goal is to come up with an implementation strategy that enhances code quality without hindering the development process or excluding new contributors.

[robyngit]

Update: The dev team thoroughly discussed the proposal for pre-commit hooks in our repositories. We've decided on a strategy that aligns with our development practices and addresses the concerns raised.

In summary, the team has agreed on the following points:

• It's important to use a consistent strategy across all our repositories.
• Running tests on every commit is overly burdensome for developers. This approach can hinder our workflow due to the time it takes tests to run in some code bases, configuration complexities, and platform-specific issues.
• There is value in being able to push incomplete or 'work-in-progress' code to feature branches. This practice is crucial for collaboration and for indicating ongoing progress in a feature.
• The current GitHub action setup, which runs tests on every pull request and prevents merging if tests fail, is an effective way to maintain code quality. This has an advantage over testing locally because it ensures that tests are run in a consistent environment with a fresh codebase.

Based on these points, we have decided to implement the following strategy:

• Instead of pre-commit hooks, we will implement pre-push hooks on the develop and main branches only. This will add an extra layer of protection, ensuring that code with failing tests cannot be pushed to these critical branches, maintaining code quality without impeding daily development activities.
• This will be complimentary to our current GitHub actions setup.
• Running tests locally will continue to be an integral part of the development process. Developers are encouraged to run tests before pushing their code, especially when merging into develop and main. Developers are also encouraged to share their work-in-progress code on feature branches, even if tests are failing, and especially if they need help from other team members.
• For those developers who prefer automated test runs at the commit stage, the option to set up a local alias for this purpose is available. This approach respects individual workflow preferences.

The focus is on maintaining high code quality without imposing unnecessary barriers on the development process. We believe that the implementation of pre-push hooks, combined with our current practices, strikes the right balance between rigor and flexibility.

Given the above, specific tasks for this issue are:

• [ ] Implement pre-push hooks for the develop and main branches specifically.
• [ ] Update the CONTRIBUTING.md file to reflect the new pre-push hook strategy, including instructions for setting up a local alias for automated test runs at the commit stage, and explanations of the philosophy behind the new approach.