Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate images into a format that is easier for collaboration #10

Closed
adrianhopebailie opened this issue Mar 20, 2019 · 17 comments
Closed
Labels
fspiop-change-request A change request for the FSPIOP API ml-ccb CCB issues or issues common to all the SIGs

Comments

@adrianhopebailie
Copy link
Contributor

The specification contain a rich set of extremely valuable diagrams.

We should find a way to store these in Git so that it is possible for change requests to be made that include modified versions of the diagrams to illustrate the changes more effectively.

This would require that they are stored in a form that is editable, preferably with minimal extra tooling.

Perhaps SVG is a good option?

@millerabel
Copy link
Member

Hi Adrian, the Mojaloop community has been using SVG for hand-drawn diagrams and we use PlantUML for sequence diagrams. Both of these formats are text-based and compatible with a Git toolchain. PlantUML in particular is excellent as a high-level actor-process-sequence markup. We use GitHub Flavored Markdown (GFM) for all narrative documents. And we have a simple toolchain that can produce static PDFs from these source files, nicely formatted, for offline reading or printing. I expect the configuration of these tools would work largely as-is for the API specification, with stylistic templates provided by the CCB to their requirements.

Have a look at the documentation files within the Mojaloop repos.

Separately, Kim Walters is exploring the use of hosted Gitbooks to improve the publication quality and provide simultaneous publication in both Web and PDF formats for Mojaloop.io. No change to the source files, just the tool chain.

@MichaelJBRichards
Copy link

It's worth noting that there seems to be an evolving (though I think as yet undefined) language for sequence diagram markup. For instance, if you strip the enclosing tags (@startuml/@enduml) from a PlantUml description it renders correctly in SequenceDiagram.org. Not sure if this is true throughout the PlantUml spec, but it certainly worked for the E2E sequence diagram. I'll look into this further.

@henrka
Copy link
Contributor

henrka commented Jun 13, 2019

I did some initial tests and it seems easier out-of-the-box to make more professional-looking diagrams in SequenceDiagram.org than in PlantUML. Below is an example using SequenceDiagram.org to draw Figure 22 in API Definition (also fixed some small issues with the current version, e.g. Payer in current version should be Payer (Agent) for clarity).

figure 22

The sequence "code" that I used in SequenceDiagram.org:
figure 22 in API Definition.txt

@henrka
Copy link
Contributor

henrka commented Jun 18, 2019

Please find some minor corrections (some styling, correct font, and an activation which should have stopped earlier) in the below source file and generated image.

figure 22 in API Definition (minor update).txt

figure 22

@henrka
Copy link
Contributor

henrka commented Jun 18, 2019

Almost the same file could also be used in PlantUML as indicated earlier by @MichaelJBRichards, with some changes in the header of the file using some skinparam settings to set up a reasonably good looking diagram.

figure 22 - plantuml.txt

Figure22

@elnyry-sam-k
Copy link
Member

Thanks Henrik for this. Let me try it out as well.

@henrka
Copy link
Contributor

henrka commented Jun 18, 2019

@elnyry Here are some additional skinparam settings to make it look more like existing diagrams:
skinparam shadowing false
skinparam defaultFontName Calibri
skinparam monochrome true
skinparam SequenceLifeLineBackgroundColor WhiteSmoke
skinparam SequenceLifeLineBorderColor Black
skinparam ActorFontStyle Bold
skinparam ActorFontSize 20
skinparam ParticipantFontStyle Bold
skinparam ParticipantFontSize 20
skinparam ParticipantBackgroundColor WhiteSmoke

Please note that the defaultFontName Calibri does not seem to work on online plantuml, only works locally for me.

@adrianhopebailie
Copy link
Contributor Author

adrianhopebailie commented Jul 10, 2019

FYI, I got the same code to work with a few modifications in Mermaid
View live editor here

I haven't played with theming to get the look to match above but the advantage of Mermaid is that it is Open Source and written in JS so would fit easily into our toolchain.

https://github.com/knsv/mermaid

There is also a GitBook plugin: https://github.com/JozoVilcek/gitbook-plugin-mermaid

@adrianhopebailie
Copy link
Contributor Author

FYI: mermaid-js/mermaid#866

@henrka
Copy link
Contributor

henrka commented Sep 4, 2019

This is probably the closest I can get using PlantUML to the existing figures without spending too much time:

Figure22

figure 22.txt

@elnyry-sam-k
Copy link
Member

@adrianhopebailie @ehenrka @MichaelJBRichards @MichaelJBRichards - any objections to closing this now?

As discussed above, we're now using SVGs for hand drawn diagrams and plantuml (with enhancements for better rendering) for sequence diagrams, as can be seen in the API Definition markdown doc. Example of a sequence diagram is here: http://mojaloop.io/mojaloop-specification/documents/API%20Definition%20v1.0.html#figure-2 and you can review the rest of the document for other architecture (or other) and sequence diagrams.

@henrka
Copy link
Contributor

henrka commented Jan 14, 2020

Some immediate comments regarding just the linked figure:

  • The height of the participant boxes should preferably be the same. Use for example participant "\nFSP" as FSP to get the same height as the Optional Switch which is in two rows.
  • Related to above, compare Peer FSP in Figure 1 to Figure 2, in Figure 1 Peer FSP is in two rows while in Figure 2 it is in one row.
  • Don't know if it is the font used, but it really looks like the word "service" is using capital I ("servIce"). At least this is an issue when using default zoom in Chrome (100%). Using for example 90% or 110% gives a correct lower case "i". Can we change the font?
  • Again, don't know if it is a font issue but "HTTP 202 (Accepted)" looks like there is no whitespace between HTTP 202 and (Accepted).
  • "Figure 2 -- HTTP GET call flow" Double "--" which seems unnecessary?
  • Use hide footbox to remove participant boxes in the footer

@elnyry-sam-k
Copy link
Member

Thanks @ehenrka , will address these.

@henrka
Copy link
Contributor

henrka commented Jan 14, 2020

I also looked at an image that is not a sequence diagram, Figure 6, and there are a lot of misspellings of "commission", and the shape depicting the Switch is very strange. The original image should be convertable to PNG in a good enough quality. I do not really see the reason for using SVG for that kind of figure.

@henrka
Copy link
Contributor

henrka commented Jan 24, 2020

Some additional comments after looking at some random sections:

  • 5.1.6.9 ATM-Initiated Cash-Out** <-- two stars (*) after section name
  • There seems to be two delays added in Figure 34, between "Payer fee is 1 USD in Payer FSP
    for ATM Cash-Out, total fee 2 USD" and "OTP is pre-generated", and similarly between "Validate OTP sent by Payee FSP, OTP OK" and "Reserve 102 USD from Payer account, 101 USD to Switch account, 1 USD to fee account". There should not be any delays. This seeems to be present in other figures as well (for example Figure 36, 46, 51, 52, 66). Please see the answer in https://forum.plantuml.net/4595/deactivating-immediately-activating-lifeline-sequence-diagrams how you can deactivate and then activate a lifeline (I can verify that it works).
  • There seems to be some extra whitespace in at least Figure 34 and 36 in "PUT /transfers/" between the "PUT /transfers/" and "".
  • Misspelling in Figure 36, "Generated OTP, "12345"" should be "Generate OTP, "12345""
  • Listing 12 is strange with double * instead of bold.
  • 6.3.4.1, "Alternative URI: PUT /parties/{Type}/{ID}/{SubId}/error**" <-- double *
  • 7.2.3.1 Regular Expression** <-- double **
  • 7.2.4.1 Regular Expression** <-- double **
  • 7.2.5.1 Regular Expression** <-- double **

@elnyry-sam-k
Copy link
Member

Thanks @ehenrka , these issues are being addressed via PR: #45 (Henk a colleague is on it)

HenkKodde pushed a commit to HenkKodde/mojaloop-specification that referenced this issue Jan 24, 2020
• Fixed - 5.1.6.9 ATM-Initiated Cash-Out** <-- two stars (*) after section name
• Fixed - two delays added in Figure 34, between "Payer fee is 1 USD in Payer FSP  for ATM Cash-Out, total fee 2 USD" and "OTP is pre-generated", and similarly between "Validate OTP sent by Payee FSP, OTP OK" and "Reserve 102 USD from Payer account, 101 USD to Switch account, 1 USD to fee account". There should not be any delays. This seems to be present in other figures as well (for example Figure 36, 46, 51, 52, 66).
• Fixed - Some extra whitespace in at least Figure 34 and 36 in "PUT /transfers/" between the "PUT /transfers/" and "".
• Fixed - Misspelling in Figure 36, "Generated OTP, "12345"" should be "Generate OTP, "12345""
• Fixed - Listing 12 is strange with double * instead of bold.
• Fixed - 6.3.4.1, "Alternative URI: PUT /parties/{Type}/{ID}/{SubId}/error**" <-- double *
• Fixed - 7.2.3.1 Regular Expression** <-- double **
• Fixed - 7.2.4.1 Regular Expression** <-- double **
• Fixed - 7.2.5.1 Regular Expression** <-- double **
HenkKodde added a commit that referenced this issue Feb 11, 2020
* - Updated version to 8.6.3
- Create Encryption document in Markdown format
- Added new Encryption document to SUMMARY.md for publishing.

* - Included package-lock.json

* - Converted the following documents in Markdown format;
	- JSON Binding Rules,
	- Generic Transaction Patterns.
- Created the following sequence diagrams required in the documents;
	- figure63a.plantuml,
	- figure64a.plantuml,
	- figure65a.plantuml,
	- figure66a.plantuml.
- Updated the following sequence diagrams;
	- figure63.plantuml,
	- figure64.plantuml,
	- figure65.plantuml,
	- figure66.plantuml.
Increase version to 8.8.1

* Format updates to accommodate auto numbering and Gitbooks display format.
Version to 8.8.1.

* Example on lifeline deactivation and activation (PayerFSP -> PayerFSP).

* Updated as per feedback received for issue #10.
• Fixed - 5.1.6.9 ATM-Initiated Cash-Out** <-- two stars (*) after section name
• Fixed - two delays added in Figure 34, between "Payer fee is 1 USD in Payer FSP  for ATM Cash-Out, total fee 2 USD" and "OTP is pre-generated", and similarly between "Validate OTP sent by Payee FSP, OTP OK" and "Reserve 102 USD from Payer account, 101 USD to Switch account, 1 USD to fee account". There should not be any delays. This seems to be present in other figures as well (for example Figure 36, 46, 51, 52, 66).
• Fixed - Some extra white space in at least Figure 34 and 36 in "PUT /transfers/" between the "PUT /transfers/" and "".
• Fixed - Misspelling in Figure 36, "Generated OTP, "12345"" should be "Generate OTP, "12345""
• Fixed - Listing 12 is strange with double * instead of bold.
• Fixed - 6.3.4.1, "Alternative URI: PUT /parties/{Type}/{ID}/{SubId}/error**" <-- double *
• Fixed - 7.2.3.1 Regular Expression** <-- double **
• Fixed - 7.2.4.1 Regular Expression** <-- double **
• Fixed - 7.2.5.1 Regular Expression** <-- double **

Co-authored-by: Sam <elnyry@users.noreply.github.com>
elnyry-sam-k pushed a commit that referenced this issue Feb 19, 2020
* - Updated version to 8.6.3
- Create Encryption document in Markdown format
- Added new Encryption document to SUMMARY.md for publishing.

* - Included package-lock.json

* - Converted the following documents in Markdown. format;
	- JSON Binding Rules,
	- Generic Transaction Patterns.
- Created the following sequence diagrams required in the documents;
	- figure63a.plantuml,
	- figure64a.plantuml,
	- figure65a.plantuml,
	- figure66a.plantuml.
- Updated the following sequence diagrams;
	- figure63.plantuml,
	- figure64.plantuml,
	- figure65.plantuml,
	- figure66.plantuml.
increase version to 8.8.1

* Format updates to accommodate auto numbering and Gitbooks display format.
Version too 8.8.1.

* Example on lifeline deactivation and activation (PayerFSP -> PayerFSP).

* Updated as per feedback received for issue #10.
• Fixed - 5.1.6.9 ATM-Initiated Cash-Out** <-- two stars (*) after section name
• Fixed - two delays added in Figure 34, between "Payer fee is 1 USD in Payer FSP  for ATM Cash-Out, total fee 2 USD" and "OTP is pre-generated", and similarly between "Validate OTP sent by Payee FSP, OTP OK" and "Reserve 102 USD from Payer account, 101 USD to Switch account, 1 USD to fee account". There should not be any delays. This seems to be present in other figures as well (for example Figure 36, 46, 51, 52, 66).
• Fixed - Some extra whitespace in at least Figure 34 and 36 in "PUT /transfers/" between the "PUT /transfers/" and "".
• Fixed - Misspelling in Figure 36, "Generated OTP, "12345"" should be "Generate OTP, "12345""
• Fixed - Listing 12 is strange with double * instead of bold.
• Fixed - 6.3.4.1, "Alternative URI: PUT /parties/{Type}/{ID}/{SubId}/error**" <-- double *
• Fixed - 7.2.3.1 Regular Expression** <-- double **
• Fixed - 7.2.4.1 Regular Expression** <-- double **
• Fixed - 7.2.5.1 Regular Expression** <-- double **

* Tiding up of sequence diagrams.

* Create "figure1_http_timeout" and "figure2_callback_timeout" sequence diagrams.
Create "Scheme Rules" in markdown.
Updated SUMMARY.md with new document for rendering in GitBooks.
Updated README.md file to reference to markdown documents.
Removed duplicated section in README.md
Updated version to 9.1.1.

* Added link to internal document reference.

* Updated Contributors.

Co-authored-by: Sam <elnyry@users.noreply.github.com>
HenkKodde added a commit that referenced this issue Feb 21, 2020
* - Updated version to 8.6.3
- Create Encryption document in Markdown format
- Added new Encryption document to SUMMARY.md for publishing.

* - Included package-lock.json

* - Converted the following documents in Markdown. format;
	- JSON Binding Rules,
	- Generic Transaction Patterns.
- Created the following sequence diagrams required in the documents;
	- figure63a.plantuml,
	- figure64a.plantuml,
	- figure65a.plantuml,
	- figure66a.plantuml.
- Updated the following sequence diagrams;
	- figure63.plantuml,
	- figure64.plantuml,
	- figure65.plantuml,
	- figure66.plantuml.
increase version to 8.8.1

* Format updates to accommodate auto numbering and Gitbooks display format.
Version too 8.8.1.

* Example on lifeline deactivation and activation (PayerFSP -> PayerFSP).

* Updated as per feedback received for issue #10.
• Fixed - 5.1.6.9 ATM-Initiated Cash-Out** <-- two stars (*) after section name
• Fixed - two delays added in Figure 34, between "Payer fee is 1 USD in Payer FSP  for ATM Cash-Out, total fee 2 USD" and "OTP is pre-generated", and similarly between "Validate OTP sent by Payee FSP, OTP OK" and "Reserve 102 USD from Payer account, 101 USD to Switch account, 1 USD to fee account". There should not be any delays. This seems to be present in other figures as well (for example Figure 36, 46, 51, 52, 66).
• Fixed - Some extra whitespace in at least Figure 34 and 36 in "PUT /transfers/" between the "PUT /transfers/" and "".
• Fixed - Misspelling in Figure 36, "Generated OTP, "12345"" should be "Generate OTP, "12345""
• Fixed - Listing 12 is strange with double * instead of bold.
• Fixed - 6.3.4.1, "Alternative URI: PUT /parties/{Type}/{ID}/{SubId}/error**" <-- double *
• Fixed - 7.2.3.1 Regular Expression** <-- double **
• Fixed - 7.2.4.1 Regular Expression** <-- double **
• Fixed - 7.2.5.1 Regular Expression** <-- double **

* Tiding up of sequence diagrams.

* Create "figure1_http_timeout" and "figure2_callback_timeout" sequence diagrams.
Create "Scheme Rules" in markdown.
Updated SUMMARY.md with new document for rendering in GitBooks.
Updated README.md file to reference to markdown documents.
Removed duplicated section in README.md
Updated version to 9.1.1.

* Added link to internal document reference.

* Updated Contributors.

* Created "Signature.md" document.
Updated "SUMMARY.md" to include "Signature.md" document.
Updated "Generic Transaction Patterns.md" and "JSON Binding Rules.md" to have the same look and feel as the other documents.
Corrected invalid index reference on "Scheme Rules.md".
Updated "README.md" to refer to "Signature.md".
Version 9.2.0.

* Changed the document version dates to "2018-03-13" as suggested.
Created "Use Cases.md" in markdown format
Update reference in "README.md" to point to "Use Cases.md"
Included "Use Cases.md" in SUMMARY.md.

Co-authored-by: Sam <elnyry@users.noreply.github.com>
HenkKodde added a commit that referenced this issue Feb 26, 2020
* - Updated version to 8.6.3
- Create Encryption document in Markdown format
- Added new Encryption document to SUMMARY.md for publishing.

* - Included package-lock.json

* - Converted the following documents in Markdown. format;
	- JSON Binding Rules,
	- Generic Transaction Patterns.
- Created the following sequence diagrams required in the documents;
	- figure63a.plantuml,
	- figure64a.plantuml,
	- figure65a.plantuml,
	- figure66a.plantuml.
- Updated the following sequence diagrams;
	- figure63.plantuml,
	- figure64.plantuml,
	- figure65.plantuml,
	- figure66.plantuml.
increase version to 8.8.1

* Format updates to accommodate auto numbering and Gitbooks display format.
Version too 8.8.1.

* Example on lifeline deactivation and activation (PayerFSP -> PayerFSP).

* Updated as per feedback received for issue #10.
• Fixed - 5.1.6.9 ATM-Initiated Cash-Out** <-- two stars (*) after section name
• Fixed - two delays added in Figure 34, between "Payer fee is 1 USD in Payer FSP  for ATM Cash-Out, total fee 2 USD" and "OTP is pre-generated", and similarly between "Validate OTP sent by Payee FSP, OTP OK" and "Reserve 102 USD from Payer account, 101 USD to Switch account, 1 USD to fee account". There should not be any delays. This seems to be present in other figures as well (for example Figure 36, 46, 51, 52, 66).
• Fixed - Some extra whitespace in at least Figure 34 and 36 in "PUT /transfers/" between the "PUT /transfers/" and "".
• Fixed - Misspelling in Figure 36, "Generated OTP, "12345"" should be "Generate OTP, "12345""
• Fixed - Listing 12 is strange with double * instead of bold.
• Fixed - 6.3.4.1, "Alternative URI: PUT /parties/{Type}/{ID}/{SubId}/error**" <-- double *
• Fixed - 7.2.3.1 Regular Expression** <-- double **
• Fixed - 7.2.4.1 Regular Expression** <-- double **
• Fixed - 7.2.5.1 Regular Expression** <-- double **

* Tiding up of sequence diagrams.

* Corrected reference in Encryption.md
Created PKI Best Practices in markdown
Recreated figure1-platforms-layout in drawio
Updated summary file for gitbooks
Updated version to 9.0.0

* Version update to 9.1.0

* Create "figure1_http_timeout" and "figure2_callback_timeout" sequence diagrams.
Create "Scheme Rules" in markdown.
Updated SUMMARY.md with new document for rendering in GitBooks.
Updated README.md file to reference to markdown documents.
Removed duplicated section in README.md
Updated version to 9.1.1.

* Added link to internal document reference.

* Updated Contributors.

* Created "Signature.md" document.
Updated "SUMMARY.md" to include "Signature.md" document.
Updated "Generic Transaction Patterns.md" and "JSON Binding Rules.md" to have the same look and feel as the other documents.
Corrected invalid index reference on "Scheme Rules.md".
Updated "README.md" to refer to "Signature.md".
Version 9.2.0.

* Changed the document version dates to "2018-03-13" as suggested.
Created "Use Cases.md" in markdown format
Update reference in "README.md" to point to "Use Cases.md"
Included "Use Cases.md" in SUMMARY.md.

* - Created "Logical Data Module" in markdown.
- Update README.md to reference markdown document "Logical Data Module".
- Update SUMMARY.md to include markdown document "Logical Data Module" in GitBooks.
- Release to 9.2.1

Co-authored-by: Sam <elnyry@users.noreply.github.com>
@elnyry-sam-k elnyry-sam-k added the ml-ccb CCB issues or issues common to all the SIGs label Feb 24, 2021
@elnyry-sam-k
Copy link
Member

As discussed above, settled on SVG format (using tools such as draw.io, etc) for hand-drawn diagrams and plantuml for traditional sequence diagrams with some formatting (as discussed above)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fspiop-change-request A change request for the FSPIOP API ml-ccb CCB issues or issues common to all the SIGs
Projects
None yet
Development

No branches or pull requests

10 participants