2017-09-19 Meeting notes

Date

19 Sep 2017

7am PST

Meeting Link

https://meetings.webex.com/collabs/#/meetings/detail?uuid=M0UUCAJ68OYLPMKAROOE9PJSBW-3O29&rnd=897752.12165

Toll: +1-415-655-0001

Number:  199 948 686 

Attendees

• Josh Zamor

• 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

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.

Existing Documentation

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
  • openlmis-download
    • ui-component creates base functionality
    • auth-ui adds authentication token
    • Generated documentation
  • facility-service-cache
    • $decorator
    • Generated documentation
  • program-service
    • 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

Action items