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

Page details: updating doc to new format #2263

Merged
merged 1 commit into from
Oct 31, 2023

Conversation

Garneauma
Copy link
Contributor

  • Updating doc to new data-first format
  • Minor PFT changes

@Garneauma Garneauma temporarily deployed to github-ci September 27, 2023 19:40 — with GitHub Actions Inactive
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Incomplete review.

There is a few changes and notes. Let's discuss next week.

_config.yml Outdated Show resolved Hide resolved
sites/page-details/samples/page-details-v3-fr.html Outdated Show resolved Hide resolved
sites/page-details/samples/page-details-v3-en.html Outdated Show resolved Hide resolved
sites/page-details/index.json-ld Outdated Show resolved Hide resolved
sites/page-details/index.json-ld Show resolved Hide resolved
sites/page-details/index.json-ld Outdated Show resolved Hide resolved
sites/page-details/index.json-ld Outdated Show resolved Hide resolved
sites/page-details/index.json-ld Outdated Show resolved Hide resolved
sites/page-details/index.json-ld Show resolved Hide resolved
@Garneauma Garneauma temporarily deployed to github-ci October 4, 2023 16:45 — with GitHub Actions Inactive
@Garneauma Garneauma requested a review from duboisp October 5, 2023 12:25
@duboisp
Copy link
Member

duboisp commented Oct 16, 2023

Pre-approved upon review + local testing

This is only a patch for GCWeb Jekyll, no impact on GCWeb versioning API, only content change for GCWeb.

Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Incomplete review

_includes/sites/page-details/page-details-v2-en.html Outdated Show resolved Hide resolved
_includes/sites/page-details/page-details-v2-fr.html Outdated Show resolved Hide resolved
_includes/sites/page-details/page-details-v3-en.html Outdated Show resolved Hide resolved
_includes/sites/page-details/page-details-v3-fr.html Outdated Show resolved Hide resolved
@Garneauma Garneauma temporarily deployed to github-ci October 30, 2023 18:03 — with GitHub Actions Inactive
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Reviewed and tested locally,

Other than the small change, it will be good to merge once completed.

Side note: Regarding the property "assets" in the iteration, we might split it into 2 properties like "Code sample" + "Assets" but let keep that change for later.

sites/page-details/index.json-ld Show resolved Hide resolved
@Garneauma Garneauma temporarily deployed to github-ci October 31, 2023 14:29 — with GitHub Actions Inactive
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

One little change, once completed we will be good to merge this PR.

sites/page-details/index.json-ld Outdated Show resolved Hide resolved
@Garneauma Garneauma temporarily deployed to github-ci October 31, 2023 17:41 — with GitHub Actions Inactive
Copy link
Member

@duboisp duboisp left a comment

Choose a reason for hiding this comment

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

Tested and reviewed.

This is only a gcweb-jekyll patch because it did touch some includes file where their output didn't change. There is no impact regarding the GCWeb versionning API.

@duboisp duboisp merged commit 49f2d69 into wet-boew:master Oct 31, 2023
1 check passed
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