Skip to content

Conversation

Ndpnt
Copy link
Member

@Ndpnt Ndpnt commented Mar 31, 2025

No description provided.

Copy link

netlify bot commented Mar 31, 2025

Deploy Preview for open-terms-archive-docs ready!

Name Link
🔨 Latest commit 7e5455e
🔍 Latest deploy log https://app.netlify.com/sites/open-terms-archive-docs/deploys/67ee4f91471893000863ca2d
😎 Deploy Preview https://deploy-preview-163--open-terms-archive-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Ndpnt Ndpnt requested a review from clementbiron April 1, 2025 08:17
Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

This is a tremendous improvement, congrats!!

@@ -0,0 +1,4 @@
---
title: Explanation
Copy link
Member

Choose a reason for hiding this comment

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

We have a plural consistency issue: “Tutorials” and “Guidelines” is plural, but “Explanation” and “How to” is singular.
I would be in favor of having every section title in plural.

Copy link
Member Author

@Ndpnt Ndpnt Apr 2, 2025

Choose a reason for hiding this comment

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

In diataxis, "tutorials" and "how-to guides" are plural whereas "reference" and "explanation" are singular, and it makes sense for me.
So i'm in favor of having "tutorials", "how-to guides" and "guidelines" and "explanation" and "reference"
What do you think @MattiSG @clementbiron?

Copy link
Member

Choose a reason for hiding this comment

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

I personally find it strange that these are singular and would prefer to have everything plural.

Copy link
Member Author

Choose a reason for hiding this comment

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

After IRL discussion, let's go with all plural

>}}
```yaml
description: >
Copy link
Member

Choose a reason for hiding this comment

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

Why do we add the key? Other examples give only the value as example.

Copy link
Member Author

Choose a reason for hiding this comment

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

When dealing with complex multiline examples, I find it more readable to include the key. However, for single values, it doesn’t seem necessary.

To that end, a range selector is a JSON object containing two keys out of the four that are available: `startBefore`, `startAfter`, `endBefore` and `endAfter`.
To facilitate cross-service comparisons and ensure consistency, a standardized list of term types is maintained in a [dedicated repository](https://github.com/OpenTermsArchive/terms-types).

Please note, the terms type may differ from the exact name provided by the service, but it should align with the underlying commitment. For example, some providers might call “Terms and Conditions” or “Terms of Use” what some others call “Terms of Service”.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Please note, the terms type may differ from the exact name provided by the service, but it should align with the underlying commitment. For example, some providers might call “Terms and Conditions” or “Terms of Use” what some others call “Terms of Service”.
Please note that the terms type may differ from the exact name provided by the service, but it should align with the underlying commitment. For example, some providers might call “Terms and Conditions” or “Terms of Use” what some others call “Terms of Service”. The standardized name should be used.

Comment on lines +66 to +68
- a CSS selector string. See the [CSS Selectors specification](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors)
- a range selector object. See the [range selector]({{< relref \"#range-selector\" >}}) section
- an array of those`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- a CSS selector string. See the [CSS Selectors specification](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors)
- a range selector object. See the [range selector]({{< relref \"#range-selector\" >}}) section
- an array of those`
- A CSS selector string. See the [CSS Selectors specification](https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_Selectors).
- A range selector object. See the [range selector]({{< relref \"#range-selector\" >}}) section.
- An array of those.`

Comment on lines +211 to +215
To capture content starting from and including a privacy section up until but excluding the footer:

```sh
npm run lint [$service_id [$another_service_id …]]
```json
{
"startBefore": "#privacy-section",
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To capture content starting from and including a privacy section up until but excluding the footer:
```sh
npm run lint [$service_id [$another_service_id …]]
```json
{
"startBefore": "#privacy-section",
To capture content starting from and including a hypothetical “European Economic Area privacy” section up until the footer (but excluding the footer):
```json
{
"startBefore": "#privacy-eea",

</ol>
</div>

<!-- Sub Navigation -->
Copy link
Member

Choose a reason for hiding this comment

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

Why isn't the breadcrumbs navigation enough to illustrate the relevance? I feel that we make the example bigger than necessary 🙂

}
```

This range selector will select the terms content between the main title and the footer element.
Copy link
Member

Choose a reason for hiding this comment

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

I love this page and the intention to provide an example! This specific one seems to me a bit risky in that it shows how to use range selectors in a case where a select: main + remove: div would be more efficient. Is there any way we can make the example clearer? 🙂

Copy link
Member

Choose a reason for hiding this comment

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

I feel that this is more of a Reference than an Explanation… This explains how to write filters more than it gives the background knowledge on their usage.

@Ndpnt Ndpnt requested a review from MattiSG April 2, 2025 08:59
Copy link
Member

@clementbiron clementbiron left a comment

Choose a reason for hiding this comment

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

I pushed the changes that I thought necessary, as seen in sync.

@clementbiron clementbiron requested a review from MattiSG April 3, 2025 08:14
Copy link
Member

@MattiSG MattiSG left a comment

Choose a reason for hiding this comment

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

What a great improvement, congrats @Ndpnt for leading this work and thanks @clementbiron too for delivering it!

@MattiSG MattiSG merged commit 652b147 into main Apr 3, 2025
5 checks passed
@MattiSG MattiSG deleted the update-service-declaration branch April 3, 2025 09:15
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.

3 participants