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

git-root: add page #6009

Merged
merged 14 commits into from
May 21, 2021
Merged

git-root: add page #6009

merged 14 commits into from
May 21, 2021

Conversation

CleanMachine1
Copy link
Member

  • 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).

For #5137

CleanMachine1 and others added 11 commits May 20, 2021 21:55
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
Co-authored-by: bl-ue <54780737+bl-ue@users.noreply.github.com>
@CleanMachine1 CleanMachine1 added the new command Issues requesting creation of a new page. label May 21, 2021
@CleanMachine1
Copy link
Member Author

The extra commits seemed to have been merged from git-extras. I may have misclicked and started a branch in VSCode from git-effort

@navarroaxel navarroaxel changed the title Git-root: add page git-root: add page May 21, 2021
@bl-ue
Copy link
Contributor

bl-ue commented May 21, 2021

@CleanMachine1 once #6002 is merged that won't be a problem.

pages/common/git-root.md Outdated Show resolved Hide resolved
pages/common/git-root.md Outdated Show resolved Hide resolved
pages/common/git-root.md Outdated Show resolved Hide resolved
Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
@marchersimon marchersimon mentioned this pull request May 21, 2021
73 tasks
CleanMachine1 and others added 2 commits May 21, 2021 15:16
Co-authored-by: Axel Navarro <navarroaxel@gmail.com>
Co-authored-by: marchersimon <50295997+marchersimon@users.noreply.github.com>
@CleanMachine1 CleanMachine1 merged commit ed0428f into main May 21, 2021
@CleanMachine1 CleanMachine1 deleted the git-root branch May 21, 2021 17:32
@sbrl
Copy link
Member

sbrl commented May 21, 2021

Remember to leave pull requests open for at least 24 hours minimum to give people a chance to comment, @CleanMachine1 :-)

@marchersimon
Copy link
Collaborator

Could we maybe integrate that into tldr-bot? Blocking merging within the first 24 hours and sending a reminder if a PR has enough approvals after 24 hours?

@bl-ue
Copy link
Contributor

bl-ue commented May 21, 2021

Sounds like a good idea. Open an issue on https://github.com/tldr-pages/tldr-bot. It'll be part of the backlog of changes we have, including a / command for set-more-info-link.py, notifying client maintainers of client spec updates via GitHub issues, etc., etc.

@CleanMachine1
Copy link
Member Author

I think 24 is a little too much. Maybe 12

@bl-ue
Copy link
Contributor

bl-ue commented May 21, 2021

We're all in different timezones, so during your proposed twelve hour wait, some of us might mostly be in bed. 24 hours ensures that regardless of timezone we've all had enough time to look about the PRs.

@marchersimon
Copy link
Collaborator

marchersimon commented May 21, 2021

I think 24 is a little too much. Maybe 12

I don't think so. In a week it doesn't matter if the page was merged 168 or 156 hours ago and to be save we should give anyone a chance to comment.

We're all in different timezones, so during your proposed twelve hour wait, some of us might mostly be in bed. 24 hours ensures that regardless of timezone we've all had enough time to look about the PRs.

I even prefer 2 days, since we can't really expect everyone to check their GitHub messages every single day.

@SethFalco
Copy link
Member

SethFalco commented May 21, 2021

Just a question, isn't the whole point of approving that people are happy with it? 🤔

Wouldn't it make more sense to:

  • Make a rule where a pull request author shouldn't merge their pull request.
  • Increase the number of required approvals.
  • Be more firm on when to approve (I've seen quite a few examples where a maintainer will approve the PR while there are unresolved discussions.)

Either one or multiple of the above. While I'm sure businesses would love to disagree 😆 , I don't think time should have part of if something should be merged or not?

@CleanMachine1
Copy link
Member Author

Maybe 3 approvals and 12 hours? since most of the time here, 3 maintainers is all PR will get

@marchersimon
Copy link
Collaborator

I also don't think increasing the minimum amount of approvals is neccesary, since it's just a minimum.

Regarding the time, I think there is no reason to rush with merging PRs. Many PRs come at the same time a day and if we would merge within 12 hours, some wouldn't have a chance to comment on them. Also, you shouldn't be punished by not being able to review just because you don't check Github regularly. As I said, in some time in won't matter at all when the PR was merged. I would still prefer 2 days in most cases.

@CleanMachine1
Copy link
Member Author

I'll be sticking to 24 hours from now. And enforcing it on others PRs.
However if the change was as small as changing -f to --force then whatever

@bl-ue
Copy link
Contributor

bl-ue commented May 21, 2021

I love getting PRs merged speedily (yesterday we had one that was merged about 3 or 4 minutes after it was created!) but I think it would be good to let 1-2 days of waiting. The reason for that is, I really don't like when PRs are merged without me getting a chance to approve 😢

@CleanMachine1
Copy link
Member Author

Is there any way we can set a time for changes bigger than +10

@CleanMachine1
Copy link
Member Author

+1 maybe not 10

@CleanMachine1
Copy link
Member Author

https://github.com/marketplace/actions/wait-sleep is this of any use/importance?

@marchersimon
Copy link
Collaborator

marchersimon commented May 23, 2021

I don't think that an action should happen after a specified amount of time, regardless of activity, approvals etc. I opened an issue in tldr-pages/tldr-bot about that.

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants