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

ip-neighbour: add page #5697

Merged
merged 3 commits into from
Apr 8, 2021
Merged

Conversation

patricedenis
Copy link
Collaborator

@patricedenis patricedenis commented Apr 6, 2021

  • The page (if new), does not already exist in the repo.
  • The page is in the correct platform directory (common/, linux/, etc.)
  • The page has 8 or fewer examples.
  • The PR title conforms to the recommended templates.
  • The page follows the content guidelines.
  • The page description includes a link to documentation or a homepage (if applicable).

@patricedenis patricedenis force-pushed the ip-neighbour-add-page branch from be2c40a to 90237f7 Compare April 6, 2021 13:22
@bl-ue bl-ue added the new command Issues requesting creation of a new page or PRs adding a new page for a command. label Apr 6, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Apr 7, 2021

@patricedenis will you please use the manned.org link here?

pages/linux/ip-neighbour.md Outdated Show resolved Hide resolved
pages/linux/ip-neighbour.md Outdated Show resolved Hide resolved
pages/linux/ip-neighbour.md Outdated Show resolved Hide resolved
pages/linux/ip-neighbour.md Outdated Show resolved Hide resolved
pages/linux/ip-neighbour.md Outdated Show resolved Hide resolved
@bl-ue
Copy link
Contributor

bl-ue commented Apr 7, 2021

@patricedenis: after we've applied all of the suggestions, let's rename this this page, the PR, and the commands from neighbour (British spelling) to neighbor (American spelling) because the command supports this and we tend to prefer American spelling because it's more standard, see #5221.

@patricedenis
Copy link
Collaborator Author

@patricedenis: after we've applied all of the suggestions, let's rename this this page, the PR, and the commands from neighbour (British spelling) to neighbor (American spelling) because the command supports this and we tend to prefer American spelling because it's more standard, see #5221.

All right, that what I just ask before so I will do it as well.

@patricedenis
Copy link
Collaborator Author

I think I will commit all the suggestions and after that I will rebase the whole to have a clean "add page PR". It is better as I understood, am I right?

@patricedenis patricedenis force-pushed the ip-neighbour-add-page branch from 1d4d057 to b370172 Compare April 7, 2021 14:58
@patricedenis patricedenis deleted the ip-neighbour-add-page branch April 7, 2021 14:59
@patricedenis patricedenis changed the title ip-neighbour: add page ip-neighbor: add page Apr 7, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Apr 7, 2021

I think I will commit all the suggestions and after that I will rebase the whole to have a clean "add page PR". It is better as I understood, am I right?

Really, it doesn't matter at all. Since we squash all of the commits into one just before merging, it will end up as one clean commit whether you yourself make a clean commit or messy commits. I recommend taking it easy and just committing as necessary, not bothering to rebase, because you can.

@patricedenis patricedenis restored the ip-neighbour-add-page branch April 7, 2021 15:02
@patricedenis
Copy link
Collaborator Author

I tried to rename the branch and it closed the PR, is that normal?

@bl-ue

Really, it doesn't matter at all. Since we squash all of the commits into one just before merging, it will end up as one clean commit whether you yourself make a clean commit or messy commits. I recommend taking it easy and just committing as necessary, not bothering to rebase, because you can.

All right.
Thanks again

@patricedenis patricedenis reopened this Apr 7, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Apr 7, 2021

I tried to rename the branch and it closed the PR, is that normal?

I see, I'm not sure if you can do that; at any rate, this is a temporary branch and so the name matters little. You could even call it asdf1234 and it wouldn't matter 😉

@bl-ue
Copy link
Contributor

bl-ue commented Apr 7, 2021

Now wait, it looks like maybe neighbour (british) is correct!! Because there's no man page for ip-neighbor, just one for ip-neighbour

Maybe you should actually revert and go back to the British spelling... 🙂

@marchersimon
Copy link
Collaborator

@bl-ue I forgot to check that too, sorry. I just looked if ip neighbor worked - which it does.

@patricedenis
Copy link
Collaborator Author

I think @bl-ue is right because I couldn't find any ip neighbor page neither on my local man pages nor on the Web (manned.org, man7.org, ...).

Should we add a note to enlighten that ip neighbor is OK to use as well?

@patricedenis patricedenis changed the title ip-neighbor: add page ip-neighbour: add page Apr 8, 2021
@bl-ue
Copy link
Contributor

bl-ue commented Apr 8, 2021

I don't really think so; they're so similar that pointing that might just be distracting.

Copy link
Contributor

@bl-ue bl-ue left a comment

Choose a reason for hiding this comment

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

Thank you, @patricedenis!

@bl-ue bl-ue merged commit ce48461 into tldr-pages:master Apr 8, 2021
@patricedenis patricedenis deleted the ip-neighbour-add-page branch April 8, 2021 13:59
@Managor Managor mentioned this pull request Jan 24, 2025
30 tasks
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 or PRs adding a new page for a command.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants