-
Notifications
You must be signed in to change notification settings - Fork 132
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
Upgrade FDC3 website to docusaurus >=3.5.2 #1371 #1418
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for fdc3 canceled.
|
Hi @robmoffat @bingenito , [this PR is raised as part of citi_hackathon event] Kindly review and suggest if any other issues. Thanks in advance. Best Regards, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the \
escape characters are still visible in some sections
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this issue still persists
#1406 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for highlighting this issue, i will fix it by using triple backticks, it is working fine in my local workspace.
@@ -1,4 +1,4 @@ | |||
# <a href='http://fdc3.finos.org/toolbox/fdc3-explained'><img src='logo.png' alt='FDC3 Explained Logo'></a> | |||
# <a href='http://fdc3.finos.org/toolbox/fdc3-explained'><img src='logo.png' alt='FDC3 Explained Logo'/> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We lost having the image within the link. Should this be changed to markdown instead of html?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is just an error that needs reverting I think. The A tag should definitely contain the image still. We may need to look for other cases that need reverting.
Well spotted @bingenito!
The equivalent markdown (for interest's sake) is:
# [![FDC3 Explained Logo](logo.png)](http://fdc3.finos.org/toolbox/fdc3-explained)
@@ -0,0 +1,62 @@ | |||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kriswest Looks like tooling generated the newly proper cased timeRange side by side with the existing lower timerange.schema.json.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apologies @bingenito I don't quite understand the comment.
- In 2.1 and next I'm only seeing the camelCased
fdc3.timeRange
schema and generated code in JS and markdown and NOT the uncasedfdc3.timerange
. - In 2.0 (only) I see the uncased
fdc3.timerange
I don't see both in one version - although I am amazed at the number of issues one mistake on the casing of a character (my bad) has caused!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kriswest
In next it is already present as lower case - https://github.com/finos/FDC3/blob/main/website/static/schemas/next/context/timerange.schema.json
This PR has generated and is now adding the camelCase side by side as it was somehow never there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gah, of course that doesn't happen to me locally as the camelcased one is on my filesystem and ensures the case is retained. Behaviour will be different on a case-sensitive file system. Note that the content of that file is actually the camel-cased version!
I dealt with that in the source files, but clearly not in the website's static folder.
Heres a PR that fixes that (I hope) @bingenito: #1420
@sivaprasadanakarla you can ignore this as it should merge into your PR fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, @sivaprasadanakarla you must be on a case-insensitive filesystem. I'll get back to you once we have this fix merged as you may need to resolve a conflict once it's in. Nothing to do right now however.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merged. @sivaprasadanakarla please merge changes from the main branch into your PR branch and then if
/website/static/schemas/next/context/timerange.schema.json still exists, delete it and make sure you only have /website/static/schemas/next/context/timeRange.schema.json
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kriswest working on it.
Hi, you may need to rebase this PR on main and re-run the checks as we've just merged a big PR for a new web-based specification. Hopefully we've not introduced too many extra links using the |
@sivaprasadanakarla please let us know if you or another participant have some time in the next few days to update this PR from main and make sure several new documents that were recently added (a big PR that had been in flight for many months just merged in) are also all good. We're very grateful that you took this task on and are keen to merge the result and get the website to a clean dependency audit! |
This update contains all the issue fixes highlighted during review.
Notes from the original PR (#1409):