Skip to content

TD Figures Update #1913

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

Merged
merged 9 commits into from
Nov 2, 2023
Merged

TD Figures Update #1913

merged 9 commits into from
Nov 2, 2023

Conversation

egekorkan
Copy link
Contributor

@egekorkan egekorkan commented Oct 31, 2023

There are some misalignments in the figures that are autogenerated and exported into png. I have fixed them in SVGs manually given the publication procedure but the toolchain should be fixed later on.

What I have done to generate the results:

  • Run npm render and regenerate the SVGs. Then push them to GitHub
  • Copyover those SVG files to publication folder
  • Manually adapt the SVG files so that errors are fixed and the organization of boxes and arrows is nicer. Due to a toolchain issue, there were missing terms in SVG (and thus PNGs). Push these SVG files to GitHub
  • Export PNGs from the previous SVG files. These overwrite the current PNGs

Status:

  • Thing: Done
  • JSON Schema: Done
  • Security: DONE
  • Forms: Done

@egekorkan egekorkan marked this pull request as ready for review October 31, 2023 15:44
@egekorkan egekorkan added the Editorial Issues with no technical impact on implementations label Oct 31, 2023
@sebastiankb sebastiankb requested a review from mmccool October 31, 2023 16:41
Copy link
Member

@JKRhb JKRhb left a comment

Choose a reason for hiding this comment

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

Thank you, @egekorkan! Just two tiny nits: It seems to me as if sometimes a space is missing between (Array of) and the respective type. Also, sometimes, the Array of in parentheses is italicized, and sometimes not.

@sebastiankb
Copy link
Contributor

This PR just updates the Figures so that they are in sync with Chapter 5.3. Approve this PR

@egekorkan
Copy link
Contributor Author

@JKRhb I have addressed your feedback. If it can be an array or string (e.g. @type), it is italic (also (Array of)). If it is always an array (e.g. required), then it not italic (also Array of)

I will also beautify the Thing diagram to remove weird placement of arrows.

@egekorkan
Copy link
Contributor Author

The Thing diagram is also updated. For preview, you can use the following URL: https://cdn.statically.io/gh/w3c/wot-thing-description/ege-image-update/publication/ver11/7-rec/Overview.html#namespaces

@mmccool
Copy link
Contributor

mmccool commented Nov 1, 2023

Met during main call, felt there were still some things to fix and we wanted a summary of the changes (we looked through the figures but it was hard to see exactly what changed due to the changes in layout). So we decided to make a resolution in the main call to give the TD editors the authority to make the necessary changes to the figures prior to REC transition. Ideally you would finalize this tomorrow in the TD coordination call.

I would be good to write a summary of the changes somewhere, however, e.g. "string changed to langString in title and description", "href added to Link and Form", etc.

As for reviews, I noticed in some places that "any type" is not formatted in black but I think it should be. Also I don't think "subclass" arrows should go downward as a general rule, but I can live with the new layout. However, as above, the group is giving you the authority to finalize this and you should do it as soon as possible.

Copy link
Contributor

@mmccool mmccool left a comment

Choose a reason for hiding this comment

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

would have been good to see a summary of changes, but ok to publish, some minor points noted elsewhere

@egekorkan
Copy link
Contributor Author

egekorkan commented Nov 1, 2023

The changes are done to the publication folder SVG files. These are:

Thing (td.svg)

  • id is mandatory --> should be optional
  • PropertyAffordance is not inheriting DataSchema
  • @type missing from InteractionAffordance

Data Schema (jsonschema.svg)

  • const, enum, default missing

Security (wotsec.svg)

  • proxy as anyURI is in SVG, but in PNG is missing
  • scheme is missing
  • In PNGs, OAuth2 was missing 3 terms

Hypermedia Control (hctl.svg)

  • op is missing

@egekorkan
Copy link
Contributor Author

Call of 02.11: @lu-zero and @mjkoster are fine with the changes. We have also updated the changes in the comment above. There is no preference regarding the placement of the parent class, general preference is to have less cluttered diagrams. We will merge it.

@egekorkan egekorkan merged commit f96b4ff into main Nov 2, 2023
@egekorkan egekorkan deleted the ege-image-update branch November 2, 2023 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Editorial Issues with no technical impact on implementations
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants