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

Finish the rename of yarn ls to yarn list. #377

Merged
merged 1 commit into from
May 19, 2017

Conversation

markstos
Copy link
Contributor

@markstos markstos commented Feb 9, 2017

Once this is merged, both #334 and #356 can be closed.

_redirects Outdated
@@ -57,3 +57,6 @@ layout: null

# Without Language
{{redirectsBase | join: newline}}

# ls was renamed to list
/en/docs/cli/ls https://yarnpkg.com/en/docs/cli/list 301
Copy link
Member

@Daniel15 Daniel15 Feb 9, 2017

Choose a reason for hiding this comment

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

Maybe do this for every language, and use a relative URI so it works properly at the preview URL:

/:language/docs/cli/ls   /:language/docs/cli/list 301

Some test cases:

(you can get this preview URL from the bottom of the pull request, where it says "deploy/netlify — Deploy preview ready!")

Copy link

@marcusmolchany marcusmolchany left a comment

Choose a reason for hiding this comment

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

Lines 11 and 13 of lang/en/docs/cli/global.md have occurrences of ls that should be list

_data/i18n/en.yml must be changed as noted in #356

_data/guides.yml has occurrences of ls

I think #277 has all the changes but has a merge conflict

@markstos markstos force-pushed the finish-renaming-yarn-ls branch from 8641e34 to 9195102 Compare February 27, 2017 00:47
@markstos
Copy link
Contributor Author

@marcusmolchany :

Lines 11 and 13 of lang/en/docs/cli/global.md have occurrences of ls that should be list

No, yarn global ls has not been renamed, so these docs should not be updated at this time. Arguably, yarn global ls should be renamed as well, but it hasn't been.

_data/i18n/en.yml must be changed

This PR did change this file as noted in #356. If there's anothe change to make to this file, I'm not sure what you are referring to.

_data/guides.yml has occurrences of ls

Those are just internal IDs, not user-facing docs

_data/guides.yml has occurrences of ls

Thanks. I missed one there. Corrected.

@markstos
Copy link
Contributor Author

@Daniel15 I've now done a force push of my proposed fix which addresses all open feedback.

If this is accepted, 3 open related PRs can be closed as well that had all partially addressed what needed to be fixed.

@ndresx ndresx mentioned this pull request Apr 20, 2017
@bestander
Copy link
Member

@markstos, sorry for a delay.
What is the status of this one? Good to merge?
Let's rebase and merge if it is.

 - yarnpkg#356 did not create redirect
 - yarnpkg#334 duplicated some work already done in yarnpkg#333 also did not update
   all necessary files.
 - yarnpkg#277 had merge conflicts that went unaddressed.

Once this is merged, all of yarnpkg#277, yarnpkg#334 and yarnpkg#356 can be closed.
@markstos markstos force-pushed the finish-renaming-yarn-ls branch from 3d03f2c to 81653b1 Compare May 19, 2017 12:50
@markstos
Copy link
Contributor Author

@bestander I'm aware no outstanding feedback to be addressed on this PR, so it seems good to merge to me. I've now rebased it onto master.

@bestander bestander merged commit 6fd484e into yarnpkg:master May 19, 2017
@bestander
Copy link
Member

Thanks, @markstos!

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.

4 participants