Best Practices for Kernel Updates
11-Dec-2008 This work mostly refers to Kernel 1 and is being updated and integrated into Best Practices for High Quality Code by Former user (Deleted) and Former user (Deleted).
Information
This documents the best practices for udpates to the kernel (core of framework). Code is currently managed using SVN so many of the practices will revolve around description of that. This will also document some of the higher level development practices that are agreed upon by the technical community.
These practices are important because user needs define the designs that are created by the UX community for Sakai. The UX community defines the tools that Sakai should include. Tool creation by developers brings needed functionality to the overall system. Updates to the framework are important/required to allow improvements to be made which support the tool developers.
Summary
- Current issues that this addresses
- Core services do not and cannot evolve with changing tool developer needs
- Core services often do not meet tool developers needs and can be more of a hindrance than a help
- Ownership of many kernel services rest on a single developer or is unclear and presents a blockade to updates and high risk when issues occur in the service
- No clear or well defined processes for kernel updates other than those with commit access can do whatever they want
- Developers (beginners and veterans) are unsure of who to ask about tool needs and proposed changes to kernel services
- Unclear who will define a direction for the kernel (core service) and how quality is maintained
- Critical framework code is untested and therefore unreliable and is often changed without evaluating the impact of the changes
- Similar to many of the issues that the kernel release addresses
- Goals (of using these practices)
- Tool developers have the functionality that is needed or it is added in a timely way
- Core service, kernel, and framework stability (better for everyone)
- Better Saaki application performance and ability to handle load (better for admins)
- More service interface stability and reduction of breaking changes (better for tool developers)
- Support for large scale improvements/updates in the kernel which are not disruptive (better for developers)
- Diverse control over the framework to ensure good development practices (better for developers)
- Benefits
- A clearly defined team of people to work on and ensure quality in the Sakai kernel
- Higher quality and reliability for Sakai services (and therefore the product as a whole)
- Better understanding of how changes to the kernel are made and approved (for veterans and beginners in the community)
- Definition of best practices and processes around the kernel
- A well defined community process for working within the most critical parts of the codebase
Best practices for Sakai Kernel updates
Kernel Services (components)
- The kernel services should be clearly defined and listed. This list should be kept up to date.
- The kernel services should be considered as the foundation on which Sakai is built and maintained carefully.
- The kernel services should be seen as supporting the tool developers. It is critical that they are easy to use, helpful, and extendable so that they do not get in the way of the tool development community.
Kernel team
- A kernel team should be formed which approves and monitors changes to the kernel
- This team should consist of trusted members of the technical community and at least one junior member (to be mentored)
- All members should have equal representation in the group
- Members should have demonstrated ability to understand the kernel, time to work on it (prefereably assigned by their institution), and self-motivation to join the team
- Ideally members should be chosen by some other group/person and confirmed by the current kernel team
- Members may leave the group at any time and should be allowed to rejoin
- The list of active members should be updated as needed
- Inactive members should be removed after a certain period of inactivity (2 months?).
- Inactive members may rejoin if they have time to work on the kernel again
- The team should consist of some minimum number of members (4?) but there will be no maximum unless decision making becomes a problem, if that happens we will deal with it then
- Kernel team practices and decisions should be documented publically and mentioned on the public lists
- This should happen well before any mandatory changes are made to the codebase or best practices are enforced
Kernel code changes and updates
- All kernel development should happen in branches
- Branches should be created for anyone who asks
- Branch creation should be timely (ideally within one day)
- Team branches should be encouraged to reduce conflicts and increase collaborative overview
- This does not have to apply to simple fixes (like pom/classpath fixes), that would just cause clutter and slow down everyone needlessly
- The kernel team should be allowed to decide on a case by case basis what fixes are too simple to require a branch
- Branches should be created for anyone who asks
- Large scale changes which require access to many projects in Sakai should be managed with a branch as well. The large scale changes should be merged and tested outside of the main svn. Once the changes are shown to be safe, they should be merged in with a single commit action by a branch manager
- An example of this would be a change to an interface which will no longer be supported (i.e. deprecated). This change would require access to many projects to fix the calls to the interface so they are using the supported method
- Branch merges should be approved by the owners of that project and should include the kernel team
- Approval should require some percentage of members (70%?) to agree
- Dissention and reasons should be noted in the svn merge commit message
- Owners are responsible for responding to a merge request in a resonable amount of time (1 week?)
- If the owner does not respond within the reasonable amount of time then the merge decision is made by the framework team
- All branches which are being merged should be mentioned to the community before they are merged (via the dev list probably)
- This is not a community vote but is more of a way to let people know what is happening and keep a public record
- Community member objections should be considered before the kernel changes are committed but they do not have to be addressed
Code Change Process Example
- This is an example of the process (as suggested by the rest of this document) that would allow for higher quality code, control over added features, stability in trunk, and code reviews
- Developer(s) publicly mention an idea to the team and community (optional but highly recommended)
- Discussed ideas are unlikely to be rejected so it is prudent to discuss ideas before working on them
- Developer(s) request a code branch from the branch manager to implement/work on their idea (all requests are granted and should be granted in a timely manner)
- Developer(s) with commit access may create their own branches but should not skip the first step (discussion)
- Developer(s) write the code in a branch until they are done
- They may have to merge in other changes to the code they are working on if merges take place while they are developing, it should not be the responsibility of the branch manager to resolve conflicts, instructions should be provided which explain how the merges are done
- Developer(s) announce they want to have their branch merged in (this should be public but it is probably a good idea to copy svn@collab.sakaiproject.org)
- Branch manager asks the kernel team to please look at the branch (before it is merged)
- Kernel team evaluates the branch (code review) and approves or suggests ways to fix the code
- This may include rejection of the merge if it was not discussed publicly before work began and/or does not fit the goals of the community
- The kernel team may help correct the code if they have time and the Developer(s) grant them permission
- This cycle continues until the branch is acceptable to the team
- Kernel team may suggest QA testing is required before merging thus it may go to QA at this point
- Branch manager receives approval from the team (or QA) and merges the branch into trunk
- NOTE: Developer defined - anyone including a member of the team or the main committer for kernel code
- NOTE: JIRA should probably be used to track this process and it might even be worth creating a workflow
Kernel commit access and ownership
- No one should have global commit access except the Sakai release management team
- The release managers are a trusted group which uses trusted accounts
- If they are also developers then they should have a separate account which they use for committing their development code
- All pieces of the kernel (e.g. user project) should IDEALLY be owned by at least 2 persons representing 2 different institutions
- This rule can be broken if an inadequate number of people are willing to take ownership but the community should strive to find owners for all pieces of the kernel
- Branch merges are conducted by the owners or the release manager
- In most circumstances it is desirable to have the release manager perform the merge
- It should be verified that the merge will build before the merge is actually committed
- Global commit access may still be necessary for especially large changes. This should be allowed but should only happen for limited periods (e.g. one month).
Use of existing projects/code
- Sakai should try to minimize the amount of code which is maintained by the community
- If an existing stable open source project provides the functionality we need, we should use it
Programming Best Practices
Kernel service interfaces (APIs)
- Kernel interfaces should have up to date and comprehensive Javadocs.
- Kernel interfaces which are expected to have local/multiple implementations should be designed to minimize required development work
- Ideally the number of methods to implement should be minimized and additional capabilities and flexibility should be added in via additional interfaces
- Logical division is critical and implementers should only need to implement the parts that they need. Interfaces should be split up so that it is easy to only implement what is needed
- Interfaces which are split up should use a common parent interface so that it is easy to refer to the implemented set of interfaces via the common root
- Services should be designed in a way that is easy to extend and should be divided into multiple interfaces along logical boundaries.
- Changes to existing interfaces should require deprecation
- Deprecated methods should have explanations of the deprecation and should include a tentative date for removal.
- Deprecated methods should stay in interfaces for as long as possible. Recommend no more than 1 year and no less than 3 months.
- Deprecated methods should log warnings for methods that are no longer correctly supported
- Deprecated methods should throw exceptions when they are not functional
- Interfaces should expose as few dependencies as possible.
- Interfaces should use IDs instead of full objects where possible
- Interfaces should include efficient methods to support limited returns of objects, sorting, and paging
- Naming convention for service interfaces?
Providers
- Providers should be supported in as many kernel services as possible.
- Pull type and push type providers should be supplied to make enhancement of Sakai as easy as possible.
- Providers should adhere to the Service Interfaces guidelines.
- Service and domain objects (model objects) should be easily extendable
- Any service or domain object interfaces which clients are expected to use should include a conservative "base" implementation class
- One way to think about this is as an "abstract" class without the "abstract" attribute.
- Allows developers focus on the methods which they're actually interested in and give them some protection against interface changes over time.
- Any service or domain object interfaces which clients are expected to use should include a conservative "base" implementation class
- The kernel should support delegation/plugins as much as practical to allow for expansion of functionality without requiring institutions to make local code changes.
Automated Testing
- All kernel services must include programmatic unit testing and/or integration testing.
- Tests should ideally be reviewed by at least one other developer
- Services should ideally include stubs which are accurate representations of how the service would operate.
- Stubs should use test data which is returned by the stub and located in a separate static class
- Stubs should be located in the "sakai-mocks" project
Other Best Practices
- The kernel team should follow and encourage other Sakai best practices as well
- The list of best practices includes those listed in this proposal, and the ones at these links
Terminology
- Kernel/Framework/Core services - the main services (APIs, IMPLs, UTILs) that make up the Sakai framework (e.g. UserDirectoryService). This does not include any tools or webapps.
- API/Interface - Java interfaces
- Kernal/Framework Pieces - A subproject which makes up part of the framework