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

docs: update deprecated link to shared flags #15722

Merged
merged 3 commits into from
Jan 5, 2024

Conversation

EvilKarter
Copy link
Contributor

Summary
In the file https://github.com/GoogleChrome/lighthouse/blob/main/docs/readme.md#differences-from-cli-flags is a link in the section "Differences from CLI flags" which points to a commit from over 6 years ago.

The linked information is outdated.

I understand that the commit was included in the link because then the highlighting of the lines will work correctly in the future when changes are made to this file.

This solution is also only a band-aid solution, because if changes are made to the linked file, this link may have to be adapted again.

The only thing I can think of to solve this permanently would be to move the shared flags to a separate file. But I don't know where this would have to be adjusted and it might also be better as a downstream ticket.

@EvilKarter EvilKarter requested a review from a team as a code owner January 4, 2024 09:54
@EvilKarter EvilKarter requested review from adamraine and removed request for a team January 4, 2024 09:54
Copy link

google-cla bot commented Jan 4, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Copy link
Member

@adamraine adamraine left a comment

Choose a reason for hiding this comment

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

The exact line numbers of this can change from commit to commit so it's more stable to use a permalink (i.e. what the old link was). That being said, you do highlight a problem with our docs that they can become stale pretty easily.

I'm not aware of a linking method that gives us the best of both worlds so for now maybe we just use this link:

export interface SharedFlagsSettings {

docs/readme.md Outdated Show resolved Hide resolved
@paulirish
Copy link
Member

The exact line numbers of this can change from commit to commit so it's more stable to use a permalink (i.e. what the old link was). That being said, you do highlight a problem with our docs that they can become stale pretty easily.

I'm not aware of a linking method that gives us the best of both worlds so for now maybe we just use this link:

https://github.com/GoogleChrome/lighthouse/blob/30e617efac5eee24d63f62f1b10f1d25adfa6cab/types/lhr/settings.d.ts#L55

how about this link: https://github.com/GoogleChrome/lighthouse/blob/main/types/lhr/settings.d.ts#:~:text=interface%20SharedFlagsSettings

@adamraine
Copy link
Member

how about this link: main/types/lhr/settings.d.ts#:~:text=interface%20SharedFlagsSettings

Perfect! Exactly what I was looking for!

docs/readme.md Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants