Story 4955: Review snapred code base
The original intention of the snapred architecture was to abstract out the data orchestration and and processing into a set of dynamic service calls to help facilitate the goals of the software in an effective manner.
This code review is not intended to tell us that the software is "good" or "bad." The code review is looking for suggestions on how to modify the code to either improve it or improve other software at the same time. While improvements to specific lines are appreciated, suggestions that are directed towards larger portions of the code (e.g. adding an object to handle something) are likely more useful. Keep in mind that it is likely you will be making changes to snapred in the future.
Expectations of reviewers:
- Reviewers will return their comments to Michael by 17:00EDT on May 3 in markdown format.
- Reviewers will keep a similar level of professionalism and comments to those that are normally found when reviewing code for a PR.
- Comments should be annotated by charge number below. Reviewers can comment on any of the charges, but they are requested to comment on their charge and those in the "General" section.
- Reviewers may use their own judgment on how much time to spend on this activity.
Mantid Experts
Reviewers: Jose, Ross, Pete
M1. What functionality does snapred reproduces unnecessarily from mantid? M2. What functionality does snapred provide that should be moved into mantid? M3. What functionality in mantid should snapred be taking advantage of? M4. How does the design of snapred recipes compare to the design mantid workflow algorithms? M5. What ui elements should snapred use? What should be added back to mantid?
Non-Mantid Experts
Reviewers: Marie, Chen, Dmitri
S1. Would you be able to add new back-end functionality to snapred? Consider testing, modifying data objects, and following the current code design. S2. How would you go about adding a test to exercise a behavior identified in a bug? S3. Are the developer guidelines clear enough on what the test options are and how to decide between them? S4. How would you modify the code to reduce overall complexity? S5. How would you modify the code to improve consistency? Is it easy to tell that multiple people are working on it, or is everything consistent?
General / Everybody
G1. What should be done to improve cohesiveness? G3. What functionality does snapred provide that should be moved into a separate package for reuse? G4. What is missing from the developer documentation? G5. Is the mvp and service level design consistent and appropriate? G6. Is the filesystem database appropriate? Are there any major pitfalls with the current implementation? G6. How can optimizations be improved? (e.g. are they relevant or premature) G7. How can caching be improved? G8. How effective is the architecture? G9. Other suggestions that is not covered by previous questions