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

Relocated inline images #101

Merged
merged 1 commit into from
Nov 29, 2022
Merged

Relocated inline images #101

merged 1 commit into from
Nov 29, 2022

Conversation

tdcox
Copy link
Contributor

@tdcox tdcox commented Nov 25, 2022

Signed-off-by: Terry Cox terry@bootstrap.je

Changes

Remove need to use scripting to make image paths compatible with Docsy for publication.

Fixes cdevents/cdevents.dev#27

Submitter Checklist

As the author of this PR, please check off the items in this checklist:

Copy link
Contributor

@afrittoli afrittoli 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 @tdcox - Could you update links to the primer too?
There is one in the README at least 🙏

Updated links to primer

Signed-off-by: Terry Cox <terry@bootstrap.je>
@tdcox
Copy link
Contributor Author

tdcox commented Nov 25, 2022

Could you update links to the primer too?

@afrittoli Done.

Copy link
Contributor

@afrittoli afrittoli 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 for the update.
Some links needs to be updated.

@@ -80,7 +80,7 @@ The specification is structured in two main parts:
dedicated document in the spec.

For an introduction see the [CDEvents README](README.md) and for more background
information please see our [CDEvents primer](primer.md).
information please see our [CDEvents primer](/docs/primer).
Copy link
Contributor

Choose a reason for hiding this comment

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

This link may work on the website, but not on github

@@ -190,7 +190,7 @@ defined in the [vocabulary](#vocabulary):
All event types should be prefixed with `dev.cdevents.`. One occurrence may
have multiple events associated, as long as they have different event types.
*Versions* are semantic in the *major.minor.patch* format. For more details about versions
see the the see [versioning](primer.md#versioning) documentation.
see the the see [versioning](/docs/primer#versioning) documentation.
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -211,7 +211,7 @@ defined in the [vocabulary](#vocabulary):
to the same application.

When selecting the format for the source, it may be useful to think about how
clients may use it. Using the [root use cases](./primer.md#use-cases) as
clients may use it. Using the [root use cases](/docs/primer#use-cases) as
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@@ -256,7 +256,7 @@ defined in the [vocabulary](#vocabulary):
- Description: The version of the CDEvents specification which the event
uses. This enables the interpretation of the context. Compliant event
producers MUST use a value of `0.1.1` when referring to this version of the
specification. For more details see [versioning](primer.md#versioning).
specification. For more details see [versioning](/docs/primer#versioning).
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto

@tdcox
Copy link
Contributor Author

tdcox commented Nov 27, 2022

@afrittoli Yes, unfortunately we have reached that point where it is no longer possible to reconcile the differences between the native GitHub environment and the Docsy environment. In all such cases, the links should be formatted to work on the system of record, which is cdevents.dev, in Docsy format.

Since the only people working directly on the spec repo will be contributors and maintainers, they will already be familiar with the location of these documents within the local repo.

@afrittoli
Copy link
Contributor

@afrittoli Yes, unfortunately we have reached that point where it is no longer possible to reconcile the differences between the native GitHub environment and the Docsy environment.

I didn't realise we would run into this issue when you presented the different options to solve the issue with the path to the images. I tried the option of having images in the same folder as the markdown files, but that doesn't really work for me. It might work if primer.md was called _index.md but that doesn't really fit with having multiple markdown files.

In all such cases, the links should be formatted to work on the system of record, which is cdevents.dev, in Docsy format.

If only publish one version to the website, a have the other version link to github, then we'll have broken links for everyone except those on the latest version. Today it's only an issue for primer.md but it may become an issue for other docs if we wanted to include more pictures, unless we decided that we cannot use pictures in the spec files.

I see the following options:

  • use an absolute link in versions published to cdevents.dev, use a github format relative link for versions only available via GitHub
  • replace the git submodule with a sync script (that's what we do for Tekton) that will overwrite links
  • maintain a -docsy version of each branch we publish to cdevents.dev. Use automation to update that branch whenever the corresponding branch is updated. Update the website to pick content from the -docsy branch(es)
  • decide we'll only ever host images in primer.md and possibly relocated that document to the website.

@tdcox
Copy link
Contributor Author

tdcox commented Nov 28, 2022

@afrittoli We are about to see multiple similar issues as we add additional documentation to the site, independent from the requirement for images. I think that the real question is whether it is appropriate to try to maintain documentation outside the website repo, and require it to be functionally independent from the website itself?

I would suggest that the risk is that a lot of brittle work-arounds and scripts are likely to start accumulating, resulting in a high maintenance overhead, for what should be a trivial application.

It might be much easier to just move all of the spec documents into cdevents.dev, and use the spec repo for the schemas and code. Versioning can be managed with working branches.

We need a decision quickly, however, as we are running out of time to complete the remaining work in this refresh and this is a blocker.

@afrittoli
Copy link
Contributor

It might be much easier to just move all of the spec documents into cdevents.dev, and use the spec repo for the schemas and code. Versioning can be managed with working branches.

I don't think this would solve the issue with links to images.

@afrittoli
Copy link
Contributor

We need a decision quickly, however, as we are running out of time to complete the remaining work in this refresh and this is a blocker.

I don't think this is a blocker for any other work, but I'm ok to merge this in the interest of avoiding rebase of other PRs.
We can address broken links later.

Copy link
Contributor

@afrittoli afrittoli left a comment

Choose a reason for hiding this comment

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

Having the primer in a dedicated folder sounds like a good idea.

@afrittoli afrittoli merged commit 2c9fa15 into cdevents:main Nov 29, 2022
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.

Image paths within documentation
2 participants