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

Terminology Design tools integration #177

Merged
merged 25 commits into from
Dec 4, 2023
Merged

Terminology Design tools integration #177

merged 25 commits into from
Dec 4, 2023

Conversation

Ca5e
Copy link
Member

@Ca5e Ca5e commented Oct 15, 2023

I've compared the changes in my fork to the master branch instead of the use-tev2 branch as I saw some work already being done on removing TEv1 documentation within the master branch.
Although deployment is done quite differently compared to before, I believe these changes should work with the current setup of the framework repository. My deployment (fork) can be seen here https://ca5e.github.io/framework/docs/essifLab.

@Ca5e Ca5e requested a review from RieksJ October 15, 2023 12:32
Copy link
Contributor

@RieksJ RieksJ left a comment

Choose a reason for hiding this comment

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

Changes seem ok.
What should be blocking for merge is that we do not yet have a HRG generated, so merging it to the eSSIF-Lab repo would mean that people browsing the site would no longer see a glossary. So we should wait until either we have a version of the glossary that we can use and publish from the repo, or there is an HRGT that would generate one.

@Ca5e Ca5e requested a review from RieksJ November 30, 2023 12:28
@Ca5e
Copy link
Member Author

Ca5e commented Nov 30, 2023

I've added the first version of the HRGT to my fork. Please have a look https://ca5e.github.io/framework/docs/essifLab-glossary. Merging should be possible.

Copy link
Contributor

Choose a reason for hiding this comment

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

I have modified the MRGRef tag spec to reflect that converter="<converter>" is optional. Of course, if it is omitted, that would imply that it be specified either on the command-line or in the configuration file (as is currently done). It gets rid of the 'ugly' converter="" text.

Copy link
Member Author

Choose a reason for hiding this comment

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

I think it may be useful to keep a simplified version as the default for using essiflab as the converter value.

So when using {% hrg="" converter="essiflab" %}.
We get:

### [{{glossaryTerm}}]({{navurl}})\n
{{glossaryText}}\n

Instead of the more complex converter that's actually used:

### [{{#if glossaryTerm}}{{glossaryTerm}}{{else}}{{capFirst term}}{{/if}}]({{localize navurl}})\n
{{#if glossaryText}}{{glossaryText}}{{else}}no `glossaryText` was specified for this entry.{{/if}}\n

This relates to writing the documentation for the HRGT and giving certain converter examples mainly. I suspect it might be more user friendly if we keep the essiflab default simple and explain the more complex one separately.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok to keep simple stuff in the tev2-specifications, but it may help them there to see examples from 'real life'.

Copy link
Contributor

Choose a reason for hiding this comment

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

Please make sure that when texts are encountered that would not work, they somehow get logged with messages that help curators to fix the problem. (I cannot tell whether or not they currently are).

Ca5e added a commit to tno-terminology-design/tev2-tools that referenced this pull request Dec 1, 2023
docs/terminology-config.yaml Show resolved Hide resolved
@RieksJ RieksJ merged commit 37151ba into essif-lab:master Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants