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

Convert Marbles to SVG and rework diagrams #1390

Merged
merged 4 commits into from
Jan 4, 2019
Merged

Convert Marbles to SVG and rework diagrams #1390

merged 4 commits into from
Jan 4, 2019

Conversation

simonbasle
Copy link
Member

Source for choice of fonts: http://www.ars-informatica.ca/article.php?article=59

Changes:

[doc] Rework marble diagrams into SVG
This commit is a large rework of the marble diagrams:

  • started from the omnigraffle original exported to SVG
  • large rework of diagrams with clearer and more consistent semantics
  • large naming rework to be more consistent with operator names
  • creation of missing marble diagrams
  • javadoc modifications to point to new diagram names
  • main javadocs no longer point to URLs but expect an embedded
    doc-files set up
  • SVG use Tahoma/Nimbus and Lucida Console/Courier New font families

[doc] Add "Contributing to SVG" README and task to copy SVG to doc-files

Said task is run as part of processResources, which gives a chance that new
users checking out the repo will see the quick documentation with diagrams
after first building the project.

@simonbasle simonbasle force-pushed the marblesToSvg branch 2 times, most recently from f1620ab to a680256 Compare October 18, 2018 16:15
simonbasle pushed a commit that referenced this pull request Oct 18, 2018
This commit is a large rework of the marble diagrams:
 - started from the omnigraffle original exported to SVG
 - removed the PNG and omnigraffle (deleted `/docs/marble/`)
 - large rework of diagrams with clearer and more consistent semantics
 - large naming rework to be more consistent with operator names
 - creation of missing marble diagrams
 - javadoc modifications to point to new diagram names
 - main javadocs no longer point to URLs but expect an embedded
 doc-files set up
 - SVG use Tahoma/Nimbus and Lucida Console/Courier New font families
simonbasle added a commit that referenced this pull request Oct 18, 2018
Said task is run as part of processResources, which gives a chance that
new users checking out the repo will see the quick documentation with
diagrams from local filesystem after first building the project.
@simonbasle
Copy link
Member Author

The diagrams are from #1377 by @thekalinga but the font family have been changed to not require any font download (and to be able to embed the SVG directly as is in the javadoc jar without mandatory text-to-path+optimization steps nor double committing of raw/optimized SVGs). Also the javadoc src changes have been reused from this PR with slight modifications and limiting the diff mostly to <img> rows.

@simonbasle simonbasle added type/documentation A documentation update wide-change labels Oct 18, 2018
@simonbasle simonbasle added this to the 3.2.2.RELEASE milestone Oct 18, 2018
@simonbasle
Copy link
Member Author

NB: now that the overall set up is finalized, we will need to validate the content of the diagrams themselves one by one vs the current release javadoc. Best way to do that is to produce a snapshot of the javadoc and visually compare it to the current released javadoc.

@simonbasle simonbasle added the status/need-investigation This needs more in-depth investigation label Oct 18, 2018
@codecov-io
Copy link

codecov-io commented Oct 18, 2018

Codecov Report

Merging #1390 into master will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1390      +/-   ##
============================================
+ Coverage     84.11%   84.24%   +0.13%     
+ Complexity     3897     3895       -2     
============================================
  Files           359      358       -1     
  Lines         29789    29419     -370     
  Branches       5522     5463      -59     
============================================
- Hits          25057    24785     -272     
+ Misses         3096     3034      -62     
+ Partials       1636     1600      -36
Impacted Files Coverage Δ Complexity Δ
.../java/reactor/core/publisher/BlockingIterable.java 71.87% <0%> (-6.25%) 7% <0%> (ø)
.../java/reactor/core/publisher/FluxContextStart.java 69.81% <0%> (-3.78%) 2% <0%> (ø)
.../reactor/core/publisher/FluxDelaySubscription.java 75.86% <0%> (-3.45%) 3% <0%> (ø)
...re/src/main/java/reactor/util/context/Context.java 82.6% <0%> (-2.01%) 17% <0%> (-2%)
...or-core/src/main/java/reactor/core/Exceptions.java 90.51% <0%> (-1.73%) 49% <0%> (-1%)
.../java/reactor/core/scheduler/ElasticScheduler.java 83.47% <0%> (-1.66%) 26% <0%> (ø)
...re/src/main/java/reactor/core/publisher/Hooks.java 95.23% <0%> (-1.09%) 35% <0%> (-6%)
...in/java/reactor/core/publisher/FluxOnAssembly.java 90.9% <0%> (-0.86%) 20% <0%> (+4%)
...rc/main/java/reactor/core/publisher/FluxRange.java 84.76% <0%> (-0.67%) 6% <0%> (ø)
...ain/java/reactor/core/publisher/FluxConcatMap.java 89.85% <0%> (-0.57%) 7% <0%> (ø)
... and 27 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e88308...e537177. Read the comment docs.

@simonbasle
Copy link
Member Author

@smaldini @thekalinga in the last commit I decided to put the SVG directly in the doc-files folder within the repo. That way, switching between master and 3.1.x doesn't show a ton on uncommitted files and there is no set up step to let the IDE correctly display diagrams from local filesystem.

The root folder can still later contain reference-specific diagrams and we can always shuffle these around a bit (but for now they are referenced by URL anyway)

@thekalinga
Copy link
Contributor

@simonbasle This is not an issue per se. But thought it would help if you too know this. In some marble diagrams (create & push as far as I remember), svgs have java code that uses Fira Code font that makes the code look much readable

This is the only place where I used font other than IBM Plex Sans. So incase you have not updated there aswell, please do so

@simonbasle
Copy link
Member Author

@thekalinga thanks for letting me know. Yeah I saw the Fira Code and replaced with "web safe fonts" Lucida Console (available by default on windows OS) with fallback to Courier New (available by default on most if not all OS)

simonbasle added a commit that referenced this pull request Dec 19, 2018
This is a better experience than in root, for having the images loaded
by IDE when first checkout/clone.
simonbasle added a commit that referenced this pull request Dec 19, 2018
The `conventions.svg` regroups all the reusable graphical conventions
used in marble diagrams.
simonbasle added a commit that referenced this pull request Dec 19, 2018
Reviewed in #1390, see PR for change details and discussion.
simonbasle added a commit that referenced this pull request Dec 19, 2018
Reviewed in #1390, see PR for change details and discussion.

Note this fixes #1399.
@simonbasle
Copy link
Member Author

Rendering of marbles inside jar tested in Eclipse (might be subpar, some arrowheads look weird, but let's see if people complain) and IntelliJ IDEA 2018.3.1 (rendering size bug has been fixed in this version)

Copy link
Contributor

@bsideup bsideup left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks really good in general! Just have a couple of things:

  • Flux#concatMapDelayError uses concatMap diagram
  • Mono.fromCompletionStage & Mono.fromFuture(Supplier) contains diagram of fromFuture (with .get() call, althought it uses whenComplete instead)

@simonbasle
Copy link
Member Author

@bsideup so far no Flux combining operator has a *DelayError variant of the diagram. I don't think it is super critical, but if it comes up a couple times more we can always add them.
The get() is from the Supplier. I added Supplier#get() to make it clearer.

@simonbasle
Copy link
Member Author

Between xmas and new year, 'tis time to merge this PR 🎅

simonbasle added a commit that referenced this pull request Dec 28, 2018
Reviewed in #1390, see PR for change details and discussion.

Note this fixes #1399.
simonbasle pushed a commit that referenced this pull request Jan 4, 2019
This commit is a large rework of the marble diagrams:
 - started from the omnigraffle original exported to SVG
 - removed the PNG and omnigraffle (deleted `/docs/marble/`)
 - large rework of diagrams with clearer and more consistent semantics
 - large naming rework to be more consistent with operator names
 - creation of missing marble diagrams
 - javadoc modifications to point to new diagram names
 - main javadocs no longer point to URLs but expect an embedded
 doc-files set up
 - SVG use Tahoma/Nimbus and Lucida Console/Courier New font families
simonbasle added a commit that referenced this pull request Jan 4, 2019
This is a better experience than in root, for having the images loaded
by IDE when first checkout/clone.
simonbasle added a commit that referenced this pull request Jan 4, 2019
The `conventions.svg` regroups all the reusable graphical conventions
used in marble diagrams.
simonbasle added a commit that referenced this pull request Jan 4, 2019
Reviewed in #1390, see PR for change details and discussion.

Note this fixes #1399.
thekalinga and others added 4 commits January 4, 2019 10:27
This commit is a large rework of the marble diagrams:
 - started from the omnigraffle original exported to SVG
 - removed the PNG and omnigraffle (deleted `/docs/marble/`)
 - large rework of diagrams with clearer and more consistent semantics
 - large naming rework to be more consistent with operator names
 - creation of missing marble diagrams
 - javadoc modifications to point to new diagram names
 - main javadocs no longer point to URLs but expect an embedded
 doc-files set up
 - SVG use Tahoma/Nimbus and Lucida Console/Courier New font families
This is a better experience than in root, for having the images loaded
by IDE when first checkout/clone.
The `conventions.svg` regroups all the reusable graphical conventions
used in marble diagrams.
Reviewed in #1390, see PR for change details and discussion.

Note this fixes #1399.
@simonbasle simonbasle merged commit 4797078 into master Jan 4, 2019
simonbasle pushed a commit that referenced this pull request Jan 4, 2019
This commit is a large rework of the marble diagrams:
 - started from the omnigraffle original exported to SVG
 - removed the PNG and omnigraffle (deleted `/docs/marble/`)
 - large rework of diagrams with clearer and more consistent semantics
 - large naming rework to be more consistent with operator names
 - creation of missing marble diagrams
 - javadoc modifications to point to new diagram names
 - main javadocs no longer point to URLs but expect an embedded
 doc-files set up
 - SVG use Tahoma/Nimbus and Lucida Console/Courier New font families
simonbasle added a commit that referenced this pull request Jan 4, 2019
This is a better experience than in root, for having the images loaded
by IDE when first checkout/clone.
simonbasle added a commit that referenced this pull request Jan 4, 2019
The `conventions.svg` regroups all the reusable graphical conventions
used in marble diagrams.
@simonbasle simonbasle deleted the marblesToSvg branch January 4, 2019 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
for/merge-with-rebase Reminder to merge the PR in rebase mode (several semantic commits) type/documentation A documentation update wide-change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants