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

pacman-s: add alias page; pacman-sync: update page #14658

Merged
merged 24 commits into from
Nov 11, 2024

Conversation

angadsgrover
Copy link
Contributor

@angadsgrover angadsgrover commented Nov 5, 2024

#14003

  • The page(s) are in the correct platform directories: common, linux, osx, windows, sunos, android, etc.
  • The page(s) have at most 8 examples.
  • The page description(s) have links to documentation or a homepage.
  • The page(s) follow the content guidelines.
  • The page(s) follow the style guide.
  • The PR title conforms to the recommended templates.

@github-actions github-actions bot added the new command Issues requesting creation of a new page. label Nov 5, 2024
@angadsgrover
Copy link
Contributor Author

@Managor and @cyqsimon
Please advise if the filename and content aligns with your expectations for #14003 . If so, I can create similar updates for the other Pacman options and submit them as separate pull requests for better organization and review.
Also please advise if you would prefer replacing the long form pages or creating new ones with similar naming scheme as to this one

@cyqsimon
Copy link
Collaborator

cyqsimon commented Nov 6, 2024

I'm a bit torn on this as well. We currently don't seem to have a convention for commands whose primary verb is a flag, as is the case with pacman. Maybe we should put this PR on hold and sort that out first.

@sebastiaanspeck
Copy link
Member

The build for this PR failed with the following error(s):

pages/linux/pacman -S.md:0: TLDR108 File name should not contain whitespace
pages/linux/pacman -S.md:0: TLDR109 File name should be lowercase

Please fix the error(s) and push again.

Why not rename it to pacman-s.md? Or pacman-S.md?

@angadsgrover
Copy link
Contributor Author

@sebastiaanspeck
I'm unable to name the file pacman -S since the naming convention restricts names to lowercase. I have since named the file pacman-s

@cyqsimon
I understand, would you want me to delete this PR then? I'll focus on the other issues and some other missing commands I wish to add to tldr.

@cyqsimon
Copy link
Collaborator

cyqsimon commented Nov 6, 2024

I understand, would you want me to delete this PR then?

No, no need for that. I was just pointing out that we probably should standardise this sort of thing.

@Managor
Copy link
Collaborator

Managor commented Nov 6, 2024

The way I see it is that the best way to handle this is to edit the commands in pacman-sync.md to match what people use. Leave the filename and title of that page as is. pacman.md already has a suggestion to check out the pacman-sync page. Edit this page to be an alias of that page.

@Managor Managor changed the title pacman -S : add page pacman-s: add page Nov 6, 2024
@angadsgrover
Copy link
Contributor Author

Just to clarify,
We should edit the shortform commands into the main pacman-sync page and add an alias for the pacman-s page with the full form commands in the pacman-s page?

@Managor
Copy link
Collaborator

Managor commented Nov 6, 2024

What do you mean with "full form commands in the pacman-s page"?

@angadsgrover
Copy link
Contributor Author

By full form commands, I mean the --sync --refresh --upgrade type commands which are currently in the pacman-sync.md page

@Managor
Copy link
Collaborator

Managor commented Nov 7, 2024

No, just make this page into an alias. See other pages like https://github.com/tldr-pages/tldr/blob/main/pages/common/trash-cli.md for example. If you want, you can use mnemonics to clarify

@tldr-bot
Copy link

tldr-bot commented Nov 7, 2024

Hello! I've noticed something unusual when checking this PR:

  • The page pages/linux/pacman-s.md seems to be a copy of pages/linux/pacman-sync.md (58% matching).

Is this intended? If so, just ignore this comment. Otherwise, please double-check the commits.

@github-actions github-actions bot added the page edit Changes to an existing page(s). label Nov 8, 2024
@tldr-bot
Copy link

tldr-bot commented Nov 8, 2024

The build for this PR failed with the following error(s):

pages/linux/pacman-s.md:
Error: Parse error on line 3:
# pacman-s- View documentation
----------^
Expecting 'TEXT', 'GREATER_THAN', got 'DASH'

Please fix the error(s) and push again.

Co-authored-by: Sebastiaan Speck <12570668+sebastiaanspeck@users.noreply.github.com>
@Managor
Copy link
Collaborator

Managor commented Nov 8, 2024

Now all that is left is converting the commands to shortform

pages/linux/pacman-sync.md Outdated Show resolved Hide resolved
pages/linux/pacman-s.md Outdated Show resolved Hide resolved
pages/linux/pacman-sync.md Outdated Show resolved Hide resolved
pages/linux/pacman-sync.md Outdated Show resolved Hide resolved
pages/linux/pacman-sync.md Outdated Show resolved Hide resolved
angadsgrover and others added 4 commits November 10, 2024 06:19
Co-authored-by: Sebastiaan Speck <12570668+sebastiaanspeck@users.noreply.github.com>
Co-authored-by: Sebastiaan Speck <12570668+sebastiaanspeck@users.noreply.github.com>
Co-authored-by: Sebastiaan Speck <12570668+sebastiaanspeck@users.noreply.github.com>
pages/linux/pacman-sync.md Outdated Show resolved Hide resolved
Co-authored-by: Managor <42655600+Managor@users.noreply.github.com>
pages/linux/pacman-s.md Outdated Show resolved Hide resolved
pages/linux/pacman-sync.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@Managor Managor left a comment

Choose a reason for hiding this comment

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

Looks good to me once the open issues have been resolved. Thanks for taking your time to go through with this.

Remember that none of this is set in stone. If you feel like the format of this page could be improved for clarity, just fire another PR.

angadsgrover and others added 4 commits November 10, 2024 14:26
Co-authored-by: Managor <42655600+Managor@users.noreply.github.com>
Co-authored-by: Sebastiaan Speck <12570668+sebastiaanspeck@users.noreply.github.com>
Co-authored-by: Managor <42655600+Managor@users.noreply.github.com>
@angadsgrover
Copy link
Contributor Author

Thank you for the corrections and feedback for the PR.
I'll try and implement similarly for the other pacman commands once this PR is completed.

@sebastiaanspeck sebastiaanspeck added the review needed Prioritized PRs marked for reviews from maintainers. label Nov 11, 2024
@kbdharun kbdharun changed the title pacman-s: add page pacman-s: add alias page; pacman-sync: update page Nov 11, 2024
Copy link
Member

@kbdharun kbdharun left a comment

Choose a reason for hiding this comment

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

LGTM, Thanks for your contribution.

@Managor Managor merged commit e1f4daa into tldr-pages:main Nov 11, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new command Issues requesting creation of a new page. page edit Changes to an existing page(s). review needed Prioritized PRs marked for reviews from maintainers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants