Skip to content

Conversation

@Steveb-p
Copy link
Contributor

@Steveb-p Steveb-p commented Feb 15, 2023

Question Answer
JIRA Ticket N/A
Versions N/A
Edition N/A

This PR ensures that HTML generated for REST API documentation remains the same across multiple re-renders.

What it basically does, it replaces the uuid function usage with hash function. Since the context provided to the hashing function does not change, it ensures that each and every time the same ID string is generated.

What we will need to see is if we ever encounter the same hash twice, as that would mean we have the same context in multiple locations. To prevent collision, all hashes are stored and rendering will fail if the same hash is generated twice.

Checklist

  • Text renders correctly
  • Text has been checked with vale
  • Description metadata is up to date
  • Redirects cover removed/moved pages
  • Code samples are working
  • PHP code samples have been fixed with PHP CS fixer
  • Added link to this PR in relevant JIRA ticket or code PR

@Steveb-p Steveb-p requested a review from DominikaK February 15, 2023 11:45

class HashExtension extends AbstractExtension
{
private $hashes = [];
Copy link
Member

Choose a reason for hiding this comment

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

is it possible for us to force higher PHP version here and use its features, like strict type in this case? I'd go for the latest PHP 8.

Copy link
Contributor

Choose a reason for hiding this comment

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

@alongosz I moved to PHP8.1+ in #1871. I can take care of this there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Technically possible, it's an internal tool.

Well, copied from an open source library afaik.

However, I'm not sure what versions of PHP our doc team have, and I'm not sure if we can just force PHP 8. PHP 7.4 would be more suitable?

Copy link
Contributor

@adriendupuis adriendupuis Feb 16, 2023

Choose a reason for hiding this comment

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

I propose #1898 as quick fix for PHP 7.4 as it doesn't work with 7.2 anymore.

For PHP 8.1+, I'll make a survey.

Choose a reason for hiding this comment

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

We're all good with at least PHP 8.1

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's merge this w/ 7.4 and I'll continue my 8.1 PR afterward.

@DominikaK DominikaK self-assigned this Feb 16, 2023
@adriendupuis adriendupuis merged commit 91a4c65 into 3.3 Feb 16, 2023
@adriendupuis adriendupuis deleted the stabilize-html-render branch February 16, 2023 12:59
adriendupuis pushed a commit that referenced this pull request Feb 16, 2023
* Stabilized render of REST HTML
* raml2html: Upgrade to PHP 7.4 (#1898)
adriendupuis pushed a commit that referenced this pull request Feb 16, 2023
* Stabilized render of REST HTML
* raml2html: Upgrade to PHP 7.4 (#1898)

(cherry picked from commit f036958)
adriendupuis pushed a commit that referenced this pull request Feb 16, 2023
* Stabilized render of REST HTML
* raml2html: Upgrade to PHP 7.4 (#1898)

(cherry picked from commit f036958)
adriendupuis pushed a commit that referenced this pull request Feb 16, 2023
* Stabilized render of REST HTML
* raml2html: Upgrade to PHP 7.4 (#1898)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants