Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

refactor(organisation): added displayTitle field, organised pages and moved random contributor component - #2624

Merged
ovflowd merged 8 commits into
nodejs:mainfrom
ovflowd:refactor/display-title-organise-links
Aug 14, 2022
Merged

refactor(organisation): added displayTitle field, organised pages and moved random contributor component#2624
ovflowd merged 8 commits into
nodejs:mainfrom
ovflowd:refactor/display-title-organise-links

Conversation

@ovflowd

@ovflowd ovflowd commented Aug 12, 2022

Copy link
Copy Markdown
Member

Check List

  • I have read the Contributing Guidelines and made commit messages that follow the guideline.
  • I have run npm run lint:js -- --fix and/or npm run lint:md -- --fix for my JavaScript and/or Markdown changes.
    • This is important as most of the cases your code changes might not be correctly linted
  • I have run npm run test to check if all tests are passing, and/or npm run test -- -u to update snapshots if I created and/or updated React Components.
  • I have checked that the build works locally and that npm run build and npm run build-storybook work fine.
  • I've covered new added functionality with unit tests if necessary.

Description

This PR adds a couple of changes by:

  • Adding displayTitle field which will be used for the localised pages (i18n)
  • Moved pages to directories that correspond to their categories
  • Moved the Random Contributor Component to Layout and made it optional

Related Issues

@ovflowd
ovflowd requested a review from benhalverson August 12, 2022 10:46
@ovflowd
ovflowd requested a review from rodion-arr August 12, 2022 10:54
@codecov-commenter

codecov-commenter commented Aug 12, 2022

Copy link
Copy Markdown

Codecov Report

Merging #2624 (e52f7c6) into main (8d360a7) will increase coverage by 0.05%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #2624      +/-   ##
==========================================
+ Coverage   84.33%   84.39%   +0.05%     
==========================================
  Files          99       99              
  Lines        1111     1115       +4     
  Branches      309      313       +4     
==========================================
+ Hits          937      941       +4     
  Misses        168      168              
  Partials        6        6              
Impacted Files Coverage Δ
src/components/Footer/index.tsx 100.00% <ø> (ø)
src/components/SideNavBar/index.tsx 100.00% <ø> (ø)
src/pages/about/governance.tsx 100.00% <ø> (ø)
src/pages/about/index.tsx 100.00% <ø> (ø)
src/pages/about/privacy.tsx 100.00% <ø> (ø)
src/pages/about/releases.tsx 100.00% <ø> (ø)
src/pages/about/security.tsx 100.00% <ø> (ø)
src/pages/about/trademark.tsx 100.00% <ø> (ø)
src/pages/about/working-groups.tsx 100.00% <ø> (ø)
src/pages/community.tsx 100.00% <ø> (ø)
... and 6 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 13, 2022
@github-actions github-actions Bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 13, 2022
@github-actions

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/2624/

@benhalverson

Copy link
Copy Markdown
Member

This will break the existing links
We should do a redirect that forwards to this new URL
For example, if I do a google search for how to install node https://www.google.com/search?q=how+to+install+node

The first result is https://nodejs.dev/learn/how-to-install-nodejs => https://nodejs.dev/en/learn/how-to-install-nodejs

@ovflowd

ovflowd commented Aug 13, 2022

Copy link
Copy Markdown
Member Author

The first result is nodejs.dev/learn/how-to-install-nodejs => nodejs.dev/en/learn/how-to-install-nodejs

I don't think this will break existing links... There are already redirects in place 🤔 but I feel like the Google Cloud deployment is broken. All the i18n changes were not deployed, somehow.

@ovflowd

ovflowd commented Aug 13, 2022

Copy link
Copy Markdown
Member Author

For some reason these deploys are failing: https://github.com/nodejs/nodejs.dev/runs/7776407250

I do not have access to the Google Cloud Panel, so I have no idea what are the errors.

@ovflowd

ovflowd commented Aug 13, 2022

Copy link
Copy Markdown
Member Author

This will break the existing links
We should do a redirect that forwards to this new URL

But yes, I will update the redirects.js with these new URL changes for this PR, but the i18n URL redirects should be working. But sadly the deployment with the i18n changes didn't succeed, as I mentioned before.

@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Aug 13, 2022
@github-actions github-actions Bot removed the create-preview Generate preview on staging.nodejs.dev label Aug 13, 2022
@github-actions

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/2624/

@ovflowd
ovflowd merged commit 732c761 into nodejs:main Aug 14, 2022
@ovflowd
ovflowd deleted the refactor/display-title-organise-links branch August 14, 2022 14:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants