-
Notifications
You must be signed in to change notification settings - Fork 238
Change Management Procedures
"Users will never forgive you for breaking their code once." John Siirola
As IDAES gains users, it becomes increasingly important to maintain backwards compatibility when making changes to the code base. Nothing frustrates users more than to find that the new "improvements" to the code broke all their existing work and that they now need to spend time to fix things.
All significant changes to the IDAES Codebase need to go through an RFC process where the process can be reviewed and any impact on backwards compatibility discussed. RFC's are tracked through GitHub issues on the idaes-pse
repo.
- All API changes
- New tools and features (new models using existing APIs and tools do not need an RFC)
- Code restructuring (including documentation and examples)
PRs that meet any of the above criteria should be rejected unless they are linked to an existing, approved RFC.
- Impacts of the proposed change, especially anything that would break existing APIs and workflows.
- Backwards compatibility measures, such as deprecation warnings and redirections.
- Interactions with or impacts upon other tools and features. IDAES code is highly modular with many interacting tools and thus changes can have far reaching effects. An example of interactions that need to be considered is how a change will interact with the IDAES scaling tools.
- For new tools, where the code belongs in the codebase (to prevent having to move it again later).
[WIP] Need a formal process for approving or rejecting RFCs. Possible outcomes:
- Approval to proceed with changes
- Delay changes until a specific IDAES release (e.g. restrict big changes to major version numbers)
- Rejection of the proposed changes
[WIP] Need to develop standards for backwards compatibility. Things to include:
- Deprecations: when they should be used, how long they should be maintained and how this should be documented in the code.
- Testing of backwards compatibility: need a set of backwards compatibility tests that cannot be (easily) changed. A separate repo might even be a good idea so that developers cannot update these as part of a PR.
- Documented examples: we are starting to build a set of documented examples, which need to remain current. Whilst we can (currently) update these easily, we should endeavor to maintain backwards compatibility with these wherever possible.
We hope in future to receive PRs from external contributors, but this adds some extra things we need to be careful of:
- Do we need to track contributors for copyright purposes?
- Need to ensure contributor has the authority to contribute the code. A good example here is that many universities assert copyright over code which students might not be aware of. We want to avoid having a case where we need to remove code from our codebase later just because someone didn't realize they did not have authority to contribute code to us.
- Following from the above, we need to be careful of proprietary data. Again, we do not want to have to scrub our codebase of data that someone contributed that happened to be protected.
- Need to be careful of infectious licensing if someone tries to add code with a different license to ours.
- We may need special processes for reviewing external code, as they will likely require more review and changes to meet IDAES standards, and some contributors may be less responsive than desired (for whatever reason).
- We will likely also need ways to grade contributions by maturity (e.g. from grad-ware to fully supported integration). This may require the creation of additional folders to segregate lower maturity code (or even separate repos).
- Set up pre-commit
- Run pytest with coverage report
- Run Pylint locally
- Update the Pyomo version
- Install Pyomo from a local Git clone
- Set up GitHub authentication with GCM
- Handle warnings in pytest