-
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 is becoming 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. However, IDAES is also continuously developing new capabilities which we would like to make available to users as soon as possible even though the interfaces and behaviors may not be fully settled yet. Further, the IDAES team would like to encourage community contributions, but recognize that external developers may not have the time, resources or knowledge to ensure their continuations are well tested and mature.
This page aims to lay out to standards and procedures IDAES will use for change management and backward compatibility whilst maintaining an open developmental environment for new features and tools.
Due to the ongoing development of the IDAES tools set and the open source nature of the project, it is necessary to recognize that different parts of the code base have different levels of maturity. Thus, there is a need to distinguish between mature ("core") code for which backward compatibility is promised and tested, and developmental or "community" code which is in a less stable state and may be subject to change.
To do this, IDAES has divided the code base into two parts based on maturity:
- core code: consisting of the
core
,models
andcommands
folders is considered mature code and must meet standards for testing and maintain backward compatibility. - developmental code; consisting of the
apps
andmodels_extra
folders is code made available to users on the understanding that it is less mature and may be subject to change or removal at any time.
Each of these categories have separate standards for change management and testing. Additionally, as the IDAES development team is relatively small, it is not possible for us to maintain the entire code base ourselves, thus this distinction also serves to delineate what the IDAES team promises to maintain from code for which we require assistance from external users to support.
The IDAES team commits to maintaining all core code, and promises that it will not be removed or changed without a sufficient grace period during which the interface is deprecated but not removed. Any changes in the core code will follow semantic versioning procedures.
All core code must have good documentation which covers all aspects of the tool/model and its use along with examples of how to use it. Where possible, tools and models should be included in example notebooks demonstrating the workflow for applying the code, which will also form the basis of the backward compatibility testing. Any example published in the IDAES examples
repository is considered to be public API (unless otherwise noted) and thus guaranteed to work across different versions of IDAES unless formally deprecated (i.e., all published examples are promised to be backward compatible).
For more information on testing of the IDAES code base, see here.
In order to ensure that the core code is working property and that backward compatibility is maintained, the following types of tests are run on a regular basis. The IDAES development team is working towards ensuring that these tests are run for all core code.
- Unit and integration testing, which tests that code functions as expected.
- Backward compatibility testing, which ensures that APIs do not change without warning and match published examples.
- Robustness and verification testing, which models and tool that code works across a wide range of inputs and provide the correct answer (verified by published experimental data where possible).
- Performance testing, which tests that changes to the code base do not degrade performance and execution time for the code.
Depending on the significance of the change, this period will be a minimum of 1 full release cycle for small changes to APIs and up to the next major version release of IDAES for significant changes.
Due to the relatively small size of the core IDAES development team, we cannot promise to help maintain all the developmental and user-contributed code. As such, all developmental code in the IDAES repository must have an identified maintainer (or maintainers) who will work with the core IDAES team to maintain the code.
Any code contributed to idaes-pse
is assumed to be for community use, and thus all code is required to have at least some documentation of how to use the tool, model or feature. For developmental code, a minimum demonstration of the application of the code to a well defined case study is required, however additional documentation is encouraged wherever possible to assist new users with applying the code. Code developers should look at this as an opportunity to expand the user base, and thus number of people willing to help support and maintain, their contribution rather than a burden.
In order to reduce the burden on developers of new code (and in recognition of the fluid state of new code), developmental code is only required to maintain a test of a minimum working example of the code (however more extensive testing is strongly encouraged to improve maintenance of the code).
The following guidelines will be used regarding ongoing support of developmental code and determining if it should be deprecated and removed.
- Developmental code will be maintained within
idaes-pse
a long as it is tested and tests pass, and a maintainer is identified for the code. - If tests begin failing, the code maintainer(s) will be notified, the tests will be
xfailed
and a deprecation warning will be placed on the code informing users that if it is not fixed then it will be deprecated after one full release cycle - If tests have not been fixed after one full release cycle, the code will be removed from
idaes-pse
(may be moved to an as yet undetermined archive).
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
repository.
- 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 code base (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
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 code base 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 code base 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 repositories).
- 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