Tldr: [RFC] Include additional Information about the command in Pull Request Template

[jsonbruce]

There are so many softwares and commands in our computer, we don't know how to distinguish some confused commands sometimes.
I think we need more Information about a new command when we review.

So, I suggest we can add additional Information in the Pull Request Template.
Needed Information like below:

• Official Page:
• Referenced Pages:

Maybe more Information more than that, welcome for more ideas.

How do you think about it? @waldyrious @agnivade @sbrl @pxgamer

[owenvoke]

I agree that this is sometimes confusing, such as when binary names are shared between different processes or for some very vague commands.

When you asked on #1958 about whether it's for the standard/standard Node package, I also thought about this sort of addition to the pull request template. But I also thought I may have just added the link myself, as in a PR, it's not always required (such as page edits or project changes).

[agnivade]

Good idea. I like it. I usually do the work of googling the tool myself and looking for the man page. But a link to the official documentation will certainly help.

[waldyrious]

Agreed, that's a good point. We already recommend this for the issues asking for new commands, but many people go straight to propose a PR so they might miss that.

My only concern that the PR template is already white verbose. We run the risk of people simply ignoring it if it looks too much like a wall of text. But we can discuss how to keep it short in the PR for this change. In principle, I like the idea. 👍

[jsonbruce]

Yeah, Thanks for your responses. Considering this point, I think we can make the PR template structured like below:

---

Basic Check

• [ ] The page (if new), does not already exist in the repo.

• [ ] The page (if new), has been added to the correct platform folder:
  common/ if it's common to all platforms, linux/ if it's Linux-specific, and so on.

• [ ] The page has 8 or fewer examples.

• [ ] The PR is appropriately titled:
  <command name>: add page for new pages, or <command name>: <description of changes> for pages being edited.

• [ ] The page follows the contributing guidelines.

References

• Official Page:

• Referenced Pages:

---

The contributors just need to append the links.
How do you think about that?

[waldyrious]

It looks fine rendered, but this is what a contributor will see filling up the text field immediately as they open a PR:

<!-- Thank you for sending a PR! -->
<!-- Please perform the following checks and check all the boxes that apply. -->
<!-- If your PR does not create a command page,
     you can remove the first two checklist items. -->
<!-- If your PR neither creates nor edits a command page (e.g. README edits, etc.)
     you can simply remove the entire checklist. -->

## Basic Check

- [ ] The page (if new), does not already exist in the repo.

- [ ] The page (if new), has been added to the correct platform folder:  
      `common/` if it's common to all platforms, `linux/` if it's Linux-specific, and so on.

- [ ] The page has 8 or fewer examples.

- [ ] The PR is appropriately titled:  
      `<command name>: add page` for new pages, or `<command name>: <description of changes>` for pages being edited.

- [ ] The page follows the [contributing](https://github.com/tldr-pages/tldr/blob/master/CONTRIBUTING.md) guidelines.

## References

- Official Page: 

- Referenced Pages: 

---

For reference, here's the current text of the template:

<!-- Thank you for sending a PR! -->
<!-- Please perform the following checks and check all the boxes that apply. -->
<!-- If your PR does not create a command page,
     you can remove the first two checklist items. -->
<!-- If your PR neither creates nor edits a command page (e.g. README edits, etc.)
     you can simply remove the entire checklist. -->

- [ ] The page (if new), does not already exist in the repo.

- [ ] The page (if new), has been added to the correct platform folder:  
      `common/` if it's common to all platforms, `linux/` if it's Linux-specific, and so on.

- [ ] The page has 8 or fewer examples.

- [ ] The PR is appropriately titled:  
      `<command name>: add page` for new pages, or `<command name>: <description of changes>` for pages being edited.

- [ ] The page follows the [contributing](https://github.com/tldr-pages/tldr/blob/master/CONTRIBUTING.md) guidelines.

To be honest, both look kinda bulky, but the new one even more so. Are we sure we won't be putting people off with too much red tape?

I'd rather look into ways to automate some of those checks (e.g. the 8 examples limit, the PR title, etc.). Then we could reduce the template to mostly linking to the contribution guide. Does this sound feasible/desirable?

[agnivade]

Agree. The 8 examples limit can be easily done if we shift to @pepa65's validator.

[sbrl]

We might even be able to automate the checking of whether the command exists already via a new linter.

[jsonbruce]

Yes, exactly. I think we can perform below basic checks by automatic tools like @tldr-bot with Travis CI.

• The page (if new), does not already exist in the repo.

• The page has 8 or fewer examples.

• The PR is appropriately titled:
  <command name>: add page for new pages, or <command name>: <description of changes> for pages being edited.

[jsonbruce]

I think it's time to enhance our tldr-lint tool first. :smile:

[waldyrious]

Ok, so shall we repurpose this issue to be about implementing those automated checks?

[jsonbruce]

Agree. But I think we can do this two things at the same time.

• Enhance PR Template as there'll be new PRs before we completed the enhancement of tldr
• Implement these new features in tldr-lint at the same time

After implementing enhanced tldr-lint, we can update PR Template by removing the three basic checks which implemented by tldr-lint.

How do you think about that?

[waldyrious]

Again, my main fear is that adding more stuff to the PR message would push it to (ironically enough!) TL;DR territory. Don't you think the text block above is too large?

[owenvoke]

I think it's a bit too large. I tested it on my latest PRs, but I think if we could cut out some of the checks (possibly by automating them) this may be easier to update.

Perhaps the linter can check for the number of lines starting with a - or something. But I don't know. :+1:

[pepa65]

I am happy to work on enhancing my linter if you'd like to use it, but the requirements aren't entirely clear at this point. Would we want different return codes for particular conditions?

[agnivade]

The return code will be same- which is 1 for error, 0 for success. You just need to print additional error messages for the new checks.

To start off with, would you like a send a PR adding your linter to the repo and changing the travis test from running tldr-lint to your script ? It already has the check for 8 examples, so that's one less item we can tick off the list right away !

[pepa65]

I can't find the travis test, and I my availability is going to be low the coming week,

[agnivade]

I will take care of it. Thanks.

EDIT- @pepa65, I think the linter checks the newer format of the pages. I ran it with the current repo and all pages fail. Could you upload the version which lints the current format ? Or maybe add a flag to switch between versions ?

[agnivade]

ping @pepa65

[pepa65]

Sorry, my linter only checks for the new syntax.

[agnivade]

Ah, okay. Then I guess it is up for grabs for anyone who is willing to revamp the linter. :smile: There is also another tangential thread to convert the pages to the new syntax. I have not seen any concrete progress on that front though. But I am fine with whichever happens first.

[pepa65]

Isn't the conversion just:

sed -i -e "1s/^# //" -e 's/^- //' -e 's/^`\(.*\)`$/    \1/' "$f"
e=$(sed '1s/./=/g;q' "$f") sed -i "2i$e" "$f"

[agnivade]

Are you saying the linter should convert the pages to the new syntax and then do the check ?

[pepa65]

Didn't mean to say that. I was responding to your:

There is also another tangential thread to convert the pages to the new syntax.

[agnivade]

Ah ok. Yes, it's not a technical matter. It's a matter of a slow transition to ensure we do not break user's clients and everybody has a chance to upgrade their clients before we start serving new pages.

[pepa65]

Wasn't the idea to keep making the old format available while starting to serve the new pages?

[agnivade]

Yes. Anyone is welcome to take it up.