2017-09-19 Meeting notes
Date
Sep 19, 2017
7am PST
Meeting Link
Toll: +1-415-655-0001
Number: 199 948 686
Attendees
@Josh Zamor (Deactivated)
@Elias Muluneh
@Paweł Gesek
@Nikodem Graczewski (Unlicensed)
@Mateusz Kwiatkowski
@Paweł Albecki (Deactivated)
@Sebastian Brudziński
@Nick Reid (Deactivated)
@Chongsun Ahn (Unlicensed)
@Ben Leibert
@Brandon Bowersox-Johnson
@Sam Im (Deactivated)
@Mary Jo Kochendorfer (Deactivated)
Goals
UI Extensible patterns
Code Review and Quality
Discussion items
Time | Item | Who | Notes |
|---|---|---|---|
30m | UI Extension Patterns followup | @Nick Reid (Deactivated) |
|
30m | Code Review and Quality | @Josh Zamor (Deactivated) |
|
Notes
UI Extension Patterns Follow Up
Previous strategy was to think we should focus on big-sky, extend anything.
Newer strategic thinking is to be more specific about where extension should occur. This is a learned piece from the amount of code that had to be forked and maintained in Malawi.
Current strategy: Add extension points where Malawi made forks
Pub/Sub application wide events
want to break apart large event logic (e.g. app wide login)
if it happens everywhere, fire an event
Decorate service/factory endpoints
if it has logic, wrap it in a factory - avoid use in controllers and directives
ui-component creates base functionality
auth-ui adds authentication token
Add new functionality to a service
Avoid business logic in Directives or Controllers
Noticed that using a decorating pattern in directives / controllers turns into a mess
Move calculations / business logic to factories or services
openlmis-download in repo openlmis-ui-components has an example of a factory holding business logic (the logic is to open the given URL in a new window)
Looking at the download factory, decorated in the auth-ui: https://github.com/OpenLMIS/openlmis-auth-ui/blob/85a0eecd80774ddce9e0a252148333124fc22b21/src/openlmis-download-auth/openlmis-download.factory.decorator.js
Since this is auth, this takes that generic download factory, and adds the OAuth2 authorization token to OpenLMIS links
Could it be moved into the directive?
Perhaps not, as the directive is the piece that works directly with the DOM.
Have had a need to extend business logic, have not yet seen the need to extend a directive.
How do we document where extensions are?
add "Decorator" to the name, so it's findable, eg. http://build.openlmis.org/job/OpenLMIS-reference-ui/lastSuccessfulBuild/artifact/build/docs/index.html#/api/openlmis-download-auth.openlmisDownloadFactory
might try to add an "implementations" section like javadocs has
Code Review
Code review documents
Recently some reviews have been:
too large. Lets reject reviews that are over a few files or a couple hundred lines
too many people, not clarity who's responsible. Lets assign 1 person (others can view, but one is assigned).
This issue has been creeping up for a while
Some reviews are very old
We have not modified our code review process for 9 months, this might be a good time to update that review process.
1 person should be assigned and responsible for the ticket in review
Use the review description field to focus the reviewer about what they are reading
UI code reviews are done for most things, but some small tweaks were missed