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.

Sign up for GitHub

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

Jump to bottom

scripts/set-alias-page.py, scripts/set-more-info-link.py: sync documentation and get_tldr_root #12744

Merged
merged 12 commits into from May 13, 2024
Merged

scripts/set-alias-page.py, scripts/set-more-info-link.py: sync documentation and get_tldr_root #12744

merged 12 commits into from May 13, 2024

Conversation

sebastiaanspeck
Copy link
Member

No description provided.

@github-actions github-actions bot added the tooling Helper tools, scripts and automated processes. label May 6, 2024
@sebastiaanspeck sebastiaanspeck mentioned this pull request May 7, 2024
Merged
vitorhcl
vitorhcl reviewed May 7, 2024
View reviewed changes
Copy link
Member

@vitorhcl vitorhcl left a comment •
edited

Choose a reason for hiding this comment

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

The current explanation of the execution directory is a bit confusing to me.

I have a suggestion, feel free to reword it.

Edit.: IMO the suggestion in my later revision is much clearer.

scripts/set-alias-page.py Outdated Show resolved Hide resolved
scripts/set-more-info-link.py Outdated Show resolved Hide resolved
vitorhcl
vitorhcl reviewed May 7, 2024
View reviewed changes
Copy link
Member

@vitorhcl vitorhcl left a comment

Choose a reason for hiding this comment

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

Actually, in my opinion, it's much clearer to explain the behavior of the script instead of trying to list some guidelines ("the script does this or that" vs. "ensure something is in this way").

If you agree with me, a rewording would be welcome if you have a better idea on the subdirectory part.

scripts/set-alias-page.py Outdated Show resolved Hide resolved
scripts/set-alias-page.py Outdated Show resolved Hide resolved
scripts/set-more-info-link.py Outdated Show resolved Hide resolved
scripts/set-more-info-link.py Outdated Show resolved Hide resolved
@sebastiaanspeck @vitorhcl
Apply suggestions from code review
d54d805 
Co-authored-by: Vítor Henrique <87824454+vitorhcl@users.noreply.github.com>
@tldr-bot

This comment was marked as outdated.

Sign in to view
@sebastiaanspeck
Copy link
Member Author

IMO we should merge this, in #12755 we will change these files again.

vitorhcl
vitorhcl approved these changes May 13, 2024
View reviewed changes
Copy link
Member

@vitorhcl vitorhcl left a comment

Choose a reason for hiding this comment

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

What about changing set-page-title-link.py too?

@sebastiaanspeck sebastiaanspeck merged commit bc34232 into main May 13, 2024
8 checks passed
@sebastiaanspeck sebastiaanspeck deleted the scripts-align branch May 13, 2024 14:21
@acuteenvy acuteenvy removed their request for review May 14, 2024 14:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kbdharun kbdharun kbdharun approved these changes

@vitorhcl vitorhcl vitorhcl approved these changes

Assignees
No one assigned
Labels
tooling Helper tools, scripts and automated processes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@sebastiaanspeck @tldr-bot @kbdharun @vitorhcl