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

Enable prettier for links file #377

Merged
merged 1 commit into from
Mar 25, 2021
Merged

Enable prettier for links file #377

merged 1 commit into from
Mar 25, 2021

Conversation

ijlee2
Copy link
Member

@ijlee2 ijlee2 commented Mar 18, 2021

Description

this enables prettier for the links file so that we can follow up and change some links in another PR

@ijlee2 ijlee2 requested a review from a team March 18, 2021 08:23
Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

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

I know you say to review this commit by commit but can we please keep the prettier and the actual link changes in different PRs to make sure that we don't miss things.

For example, this PR adds a regression where the /release/ part of the Guides link is being removed 👍

@mansona
Copy link
Member

mansona commented Mar 18, 2021

also I'm surprised by the "remove the copy in the website" because this should never have been added to the website in the first place 🤔

I can't find the PR where it was added, do you have the link?

Comment on lines +6 to +20
{
href: 'https://guides.emberjs.com/release/',
name: 'Ember.js Guides',
type: 'link',
},
{
href: 'https://api.emberjs.com',
name: 'API Reference',
type: 'link',
},
{
href: 'https://cli.emberjs.com',
name: 'CLI Guides',
type: 'link',
},
Copy link
Member Author

Choose a reason for hiding this comment

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

https://guides.emberjs.com is redirected to https://guides.emberjs.com/release and matches the pattern for API and CLI Guides below. The API and CLI links are redirected to the corresponding release pages. I don't think it's good to have a mix of patterns.

@locks locks changed the title Updated links for ember-website Enable prettier for links file Mar 25, 2021
@locks
Copy link
Contributor

locks commented Mar 25, 2021

We discussed this PR at the meeting today and we are merging the formatting changes so we can proceed with updating the actual links :)
I updated the title to better reflect the content of the PR.

@locks locks merged commit 6a9956c into master Mar 25, 2021
@locks locks deleted the update-links branch March 25, 2021 17:44
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.

3 participants